8240969: WebView does not allow to load style sheet in modularized applications #22
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently, loading a style sheet file using `WebView.getEngine().setUserStyleSheetLocation(url)` fails if the url start's with `jrt`, i.e. if the file is packaged in an application image using jlink. This is fixed with this PR.
|
Hi tobiasdiez, welcome to this OpenJDK project and thanks for contributing! We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user tobiasdiez" as summary for the issue. If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing |
6 tasks
|
Hi Tobias, |
|
@tobiasdiez are you still interested in pursuing this fix? |
|
Yes, I am. But, to be honest, it's a bit overly complicated to contribute here with the JBS bug requirement and the oracle agreement. Hopefully I'll find the time to do this next week. |
4 tasks
|
/signed The OCA should be ok now. Is there something else that needs to be done? |
|
Thank you! Please allow for up to two weeks to process your OCA, although it is usually done within one to two business days. Also, please note that pull requests that are pending an OCA check will not usually be evaluated, so your patience is appreciated! |
|
Once your GitHub account is linked to your OCA, the bot will remove the The other thing that is needed is a bug report in JBS. You can file one at bugreport.java.com. This should include a test program, ideally one that you can turn into an automated unit test. |
bridgekeeper
bot
removed
oca
|
@tobiasdiez Your OCA has now been processed and linked with your GitHub account. This can proceed once we have a bug filed in JBS with a test case. |
|
@tobiasdiez in case you missed the earlier comment, this is now ready for you to proceed. All we need is a bug report with a test case. You can file the bug at bugreport.java.com. |
|
Has this bug been finally reported ? I can do it if it didn't. |
|
Sorry for the delay, but I've finally managed to create the required bug report (internal review ID : 9063979) |
|
This has now been transferred to the JDK project as JDK-8240969. You can update the title of this PR to include the bug ID, followed by a I note that we still need a test program that fails currently, and passes with your patch. |
tobiasdiez
changed the title
|
I've changed the title accordingly. However, I've no idea how to add a test and couldn't find any unit test concerning |
Webrevs
|
|
You must have some use case that fails without this proposed fix (else why are you proposing it), right? |
|
Yes, we had this exception in our application which was (temporarily) fixed with JabRef/jabref#5497. But JabRef is 100k lines of code, so I doubt this counts as a unit test. |
|
In that case, we will need you to distill it down to a simple, standalone test program. It seems that it might not be suitable for a unit test, but we need some test to show that the fix is failing now, and works after your proposed fix. |
|
@tobiasdiez are you planning to pursue this? The review is pending a simple test case. |
|
@kevinrushforth I've now added a test. Hope it's ok like this. |
3 tasks
kevinrushforth
requested changes
Aug 13, 2020
modules/javafx.web/src/test/java/test/javafx/scene/web/MiscellaneousTest.java
Outdated
Show resolved
Hide resolved
|
/reviewers 2 |
|
@kevinrushforth |
kevinrushforth
approved these changes
Aug 14, 2020
|
@arun-joseph can you also review? |
arun-joseph
approved these changes
Aug 14, 2020
|
@tobiasdiez This change now passes all automated pre-integration checks. When the change also fulfills all project specific requirements, type
Since the source branch of this PR was last updated there have been 276 commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid automatic rebasing, please merge As you are not a known OpenJDK Author, an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@kevinrushforth, @arun-joseph) but any other Committer may sponsor as well. ➡️ To flag this PR as ready for integration with the above commit message, type |
|
/integrate |
|
@tobiasdiez |
|
/sponsor |
openjdk
bot
added
integrated
|
@kevinrushforth @tobiasdiez The following commits have been pushed to master since your change was applied:
Your commit was automatically rebased without conflicts. Pushed as commit 2aed5ad. |
33 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, loading a style sheet file using
WebView.getEngine().setUserStyleSheetLocation(url)fails if the url start's withjrt, i.e. if the file is packaged in an application image using jlink. This is fixed with this PR.Progress
Issue
Reviewers
Download
$ git fetch https://git.openjdk.java.net/jfx pull/22/head:pull/22$ git checkout pull/22