8256019: JLabel HTML text does not support translucent text colors by prsadhuk · Pull Request #1158 · openjdk/jdk

prsadhuk · 2020-11-11T08:00:09Z

Issue is a JLabel with a translucent foreground color properly renders plain text, but with HTML text, the alpha component is discarded and the text is rendered using an opaque color.
As per https://www.w3schools.com/cssref/func_rgba.asp, CSS supports rgba() to support alpha and render translucent text color
but support for rgba() is not present in JDK html text rendering.

Added support for rgba() to render translucent text color.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8256019: JLabel HTML text does not support translucent text colors

Reviewers

Sergey Bylokhov (@mrserb - Reviewer)

Download

$ git fetch https://git.openjdk.java.net/jdk pull/1158/head:pull/1158
$ git checkout pull/1158

bridgekeeper · 2020-11-11T08:00:44Z

👋 Welcome back psadhukhan! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2020-11-11T08:02:34Z

@prsadhuk The following label will be automatically applied to this pull request:

swing

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

prsadhuk · 2020-11-11T08:03:08Z

src/java.desktop/share/classes/javax/swing/plaf/basic/BasicHTML.java

+                color[i] = color[i].strip();
+            }
+            col = new Color(Integer.parseInt(color[0]), Integer.parseInt(color[1]),
+                            Integer.parseInt(color[2]), (int)(Float.parseFloat(color[3])*255));


It seems OceanTheme in Metal L&F supports only rgb

ColorUIResource CONTROL_TEXT_COLOR = new PrintColorUIResource(0x333333, Color.BLACK)

so even when Color(r,g,b,a) is used, it creates rgb color and not rgba
so alpha is obtained and passed separately.

I have already mentioned it here.

But in what place we parse "rgb(...)"? it does not seem mentioned in this method and does not depend on the c.getForeground()?

It is parsed in Parser.java#parseAttributeValue() which is being called from this method via kit.read().
It also parses rgba() and store it but JDK does not seem to support rgba() in CSS.java and also SwingUtilities2.displayPropertiesToCSS() which is being called prior to html parsing so alpha needs to be parsed separately.

mlbridge · 2020-11-11T08:06:07Z

Webrevs

mrserb · 2020-11-19T19:14:14Z

src/java.desktop/share/classes/javax/swing/text/html/CSS.java

      else if (str.startsWith("rgb(")) {
          color = parseRGB(str);
+      } else if (str.startsWith("rgba(")) {
+          color = parseRGBA(str);


Probably we need to update the spec for StyleSheet.stringToColor()

mrserb · 2020-11-19T19:16:31Z

src/java.desktop/share/classes/javax/swing/plaf/basic/BasicHTML.java

     */
    public static View createHTMLView(JComponent c, String html) {
        BasicEditorKit kit = getFactory();
+        int beginIndex = html.indexOf("rgba(");


Don't we need to implement this parsing similarly to rgb()? somewhere inside kit.createDefaultDocument() or where we parse rgb()?

The problem is the alpha color not being present in c.getForeground() so we need to parse alpha here to pass the value to displayPropertiesToCSS(). I have already mentioned it below.

As far as I understand the only place where we decode rgb() is CSS.stringToColor and that code missing the rgba() case.
Just to double-check.
If the test case TestTranslucentLabelText will be modified to use rgb() instead of rgba() then the color which was set by the user to the Component will be ignored, and the correct color from the rgb() will be used. So it does not matter that c.getForeground() contains some opaque color, it is ignored, why the similar case for rgba() does not work in the same way?

Yes, seems like overkill last time which is not needed. Reverted back unneeded code.

prsadhuk · 2020-12-02T10:19:18Z

any further comments?

mrserb · 2020-12-04T02:40:40Z

test/jdk/javax/swing/JLabel/TestTranslucentLabelText.java

+ * @test
+ * @bug 8256019
+ * @summary Verifies if JLabel HTML text support translucent text colors
+ * @run main/manual TestTranslucentLabelText


Is it necessary to make this test manual? probably we can just draw the label to the buffered image and check resulted content?

I tried before making it manual but could not effectively do that.

victordyakov · 2020-12-08T16:20:20Z

@Kizune please review

openjdk · 2020-12-09T22:48:42Z

@prsadhuk This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8256019: JLabel HTML text does not support translucent text colors

Reviewed-by: serb

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 520 new commits pushed to the master branch:

46c9a86: 8245956: JavaCompiler still uses File API instead of Path API in a specific case
baf4c1a: 8257917: [JVMCI] crash when materializing boxed values under -Xcomp
fd5f6e2: 8257986: [JVMCI] ProblemList 2 reprofile JVMCI tests
30de320: 6882207: Convert javap to use diamond operator internally
d33a689: 8256867: Classes with empty PermittedSubclasses attribute cannot be extended
e6b4c4d: 8238781: [macos] jpackage tests failed due to "hdiutil: convert failed" in various ways
b977a7b: 8257989: Error in gtest os_page_size_for_region_unaligned after 8257588
5f03341: 8052260: Reference.isEnqueued() spec does not match the long-standing behavior returning true iff it's in the ref queue
6dd06ad: 8254996: make jdk.net.UnixDomainPrincipal a record class
bd22aa5: 8229862: NPE in jdk.compiler/com.sun.tools.javac.jvm.Code.emitop0(Code.java:570)
... and 510 more: https://git.openjdk.java.net/jdk/compare/a38dd53411f896c4c307d2d55727e5d3915819e4...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

bridgekeeper · 2021-01-06T23:30:17Z

@prsadhuk This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

prsadhuk · 2021-01-13T06:57:09Z

/integrate

openjdk · 2021-01-13T06:58:18Z

@prsadhuk Since your change was applied there have been 885 commits pushed to the master branch:

0957d9e: 8259519: The java.awt.datatransfer.DataFlavor#ioInputStreamClass field is redundant
65bed64: 8253635: Implement toString() for SSLEngineImpl
c6d798c: 8259629: aarch64 builds fail after JDK-8258932
4be2173: 8259319: Illegal package access when SunPKCS11 requires SunJCE's classes
e4df209: 7018932: Drawing very large coordinates with a dashed Stroke can cause Java to hang
5f7ccce: 8226810: Failed to launch JVM because of NullPointerException occured on System.props
d6a2105: 8259343: [macOS] Update JNI error handling in Cocoa code.
c338f11: 8259349: -XX:AvgMonitorsPerThreadEstimate=1 does not work right
ccac7aa: 8258932: AArch64: Enhance floating-point Min/MaxReductionV with fminp/fmaxp
4c75d14: 8259374: Make ThreadInVMfromNative have ResetNoHandleMark
... and 875 more: https://git.openjdk.java.net/jdk/compare/a38dd53411f896c4c307d2d55727e5d3915819e4...master

Your commit was automatically rebased without conflicts.

Pushed as commit 44c8379.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Initial fix

769eb29

openjdk bot added the rfr Pull request is ready for review label Nov 11, 2020

openjdk bot added the swing [email protected] label Nov 11, 2020

prsadhuk commented Nov 11, 2020

View reviewed changes

mrserb reviewed Nov 19, 2020

View reviewed changes

prsadhuk added 2 commits November 20, 2020 03:37

Update spec

bf0c84a

Revert unneeded additional code

f98cae5

mrserb reviewed Dec 4, 2020

View reviewed changes

mrserb approved these changes Dec 9, 2020

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Dec 9, 2020

openjdk bot closed this Jan 13, 2021

openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jan 13, 2021

prsadhuk deleted the JDK-8256019 branch January 13, 2021 06:58

Conversation

prsadhuk commented Nov 11, 2020 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Download

Uh oh!

bridgekeeper bot commented Nov 11, 2020

Uh oh!

openjdk bot commented Nov 11, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mlbridge bot commented Nov 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prsadhuk commented Dec 2, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

victordyakov commented Dec 8, 2020

Uh oh!

openjdk bot commented Dec 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bridgekeeper bot commented Jan 6, 2021

Uh oh!

prsadhuk commented Jan 13, 2021

Uh oh!

openjdk bot commented Jan 13, 2021

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

prsadhuk commented Nov 11, 2020 •

edited by openjdk bot

Loading

mlbridge bot commented Nov 11, 2020 •

edited

Loading

openjdk bot commented Dec 9, 2020 •

edited

Loading