8292948: JEditorPane ignores font-size styles in external linked css-file #10054

stanio · 2022-08-27T08:54:41Z

JDK-8292948 reveals a regression by the fix for JDK-8257664 (#1759) causing font-size declarations from external style sheets to be ignored.

As far as I've traced, the problem is with the isDefined(CSS.Attribute.FONT_SIZE) test:

jdk/src/java.desktop/share/classes/javax/swing/text/html/StyleSheet.java

Lines 2825 to 2830 in 70b5b31

    
           Object doGetAttribute(Object key) { 
        
               if (key == CSS.Attribute.FONT_SIZE && !isDefined(key)) { 
        
                   // CSS.FontSize represents a specified value and we need 
        
                   // to inherit a computed value so don't resolve percentage 
        
                   // value from parent. 
        
                   return fontSizeInherit();

It misses declarations from linked styles via a resolver (AttributeSet.ResolveAttribute) attribute that happens to hold the external style sheet rules. The fix for this would be to move the implied CSS.Attribute.FONT_SIZE handling after:

jdk/src/java.desktop/share/classes/javax/swing/text/html/StyleSheet.java

Lines 2833 to 2835 in 70b5b31

    
           Object retValue = super.getAttribute(key); 
        
           if (retValue != null) { 
        
               return retValue;

and drop the isDefined() test.

[I'll prepare a jtreg test further.]

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8292948: JEditorPane ignores font-size styles in external linked css-file

Reviewers

Prasanta Sadhukhan (@prsadhuk - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/10054/head:pull/10054
$ git checkout pull/10054

Update a local copy of the PR:
$ git checkout pull/10054
$ git pull https://git.openjdk.org/jdk pull/10054/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 10054

View PR using the GUI difftool:
$ git pr show -t 10054

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/10054.diff

Fully resolve specified font-size before implying 'font-size: inherit'. The isDefined(CSS.Attribute.FONT_SIZE) test misses declarations resolved via AttributeSet.ResolveAttribute ("resolver") attribute.

bridgekeeper · 2022-08-27T08:57:06Z

👋 Welcome back stanio! 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 · 2022-08-27T09:01:15Z

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

client

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.

mlbridge · 2022-08-27T09:04:03Z

Webrevs

test/jdk/javax/swing/text/html/StyleSheet/TestExternalCSSFontSize.java

Don't rely on the current working directory.

The failure to save the image suppresses the original failure.

Simplify overall setup.

Make potential discrepancies with not resolving the base more obvious.

Use just the instance editor component.

stanio · 2022-08-28T05:17:32Z

Failure (current)	Success

openjdk · 2022-09-02T07:14:59Z

@stanio 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:

8292948: JEditorPane ignores font-size styles in external linked css-file

Reviewed-by: psadhukhan

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 169 new commits pushed to the master branch:

68da02c: 8292240: CarrierThread.blocking not reset when spare not activated
005b49b: 8293044: C1: Missing access check on non-accessible class
91d00b3: 8288473: Remove unused frame::set_pc_preserve_deopt methods
45ff10c: 8292695: SIGQUIT and jcmd attaching mechanism does not work with signal chaining library
dbec22b: 8293287: add ReplayReduce flag
b8598b0: 8291660: Grapheme support in BreakIterator
a14c3a4: 8288933: Improve the implementation of Double/Float.isInfinite
00befdd: 8292675: Add identity transformation for removing redundant AndV/OrV nodes
7169ee5: 8293477: IGV: Upgrade to Netbeans Platform 15
3dd94f3: 8292671: Hotspot Style Guide should allow covariant returns
... and 159 more: https://git.openjdk.org/jdk/compare/1500d3dfb217c2c03b925622db841310f5aedb9d...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@prsadhuk) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

stanio · 2022-09-12T05:27:30Z

/integrate

stanio · 2022-09-12T05:29:31Z

If there are no other reviewers, @prsadhuk, would you sponsor this change?

openjdk · 2022-09-12T05:29:44Z

@stanio
Your change (at version c7d8f0b) is now ready to be sponsored by a Committer.

prsadhuk · 2022-09-15T08:14:36Z

/sponsor

openjdk · 2022-09-15T08:16:43Z

Going to push as commit 1caba0f.
Since your change was applied there have been 210 commits pushed to the master branch:

eeb625e: 8290169: adlc: Improve child constraints for vector unary operations
2057070: 8293815: P11PSSSignature.engineUpdate should not print debug messages during normal operation
7376c55: 8293769: RISC-V: Add a second temporary register for BarrierSetAssembler::load_at
d191e47: 8293768: Add links to JLS 19 and 20 from SourceVersion enum constants
a75ddb8: 8293122: (fs) Use file cloning in macOS version of Files::copy method
95c7c55: 8293402: hs-err file printer should reattempt stack trace printing if it fails
211fab8: 8291669: [REDO] Fix array range check hoisting for some scaled loop iv
7f3250d: 8293787: Linux aarch64 build fails after 8292591
2a38791: 8292755: Non-default method in interface leads to a stack overflow in JShell
8351b30: 8293771: runtime/handshake/SystemMembarHandshakeTransitionTest.java fails if MEMBARRIER_CMD_QUERY is unsupported
... and 200 more: https://git.openjdk.org/jdk/compare/1500d3dfb217c2c03b925622db841310f5aedb9d...master

Your commit was automatically rebased without conflicts.

openjdk · 2022-09-15T08:17:12Z

@prsadhuk @stanio Pushed as commit 1caba0f.

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

8292948: Resolve font-size from external/linked styles

287a8ef

Fully resolve specified font-size before implying 'font-size: inherit'. The isDefined(CSS.Attribute.FONT_SIZE) test misses declarations resolved via AttributeSet.ResolveAttribute ("resolver") attribute.

openjdk bot added the rfr Pull request is ready for review label Aug 27, 2022

openjdk bot added the client client-libs-dev@openjdk.org label Aug 27, 2022

stanio added 2 commits August 27, 2022 12:55

8292948: Update copyright year

2ae1ac3

8292948: Add suggested jtreg test

f7b5c90

stanio commented Aug 27, 2022

View reviewed changes

test/jdk/javax/swing/text/html/StyleSheet/TestExternalCSSFontSize.java Outdated Show resolved Hide resolved

stanio added 7 commits August 27, 2022 20:29

8292948: Adjust @summary

153a5a8

8292948: Resolve test HTML file from the classpath

c1f6183

Don't rely on the current working directory.

8292948: Capitalize messages

2f4a5d3

8292948: Avoid failing to save image capture when editor == null

7d76f95

The failure to save the image suppresses the original failure.

8292948: Make the setUp(), run/verify() sequence invocation explicit

67f859c

Simplify overall setup.

8292948: Use bigger/different base font than the default

301aadf

Make potential discrepancies with not resolving the base more obvious.

8292948: Make captureImage() an instance method

c7d8f0b

Use just the instance editor component.

prsadhuk approved these changes Sep 2, 2022

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Sep 2, 2022

openjdk bot added the sponsor Pull request is ready to be sponsored label Sep 12, 2022

openjdk bot added the integrated Pull request has been integrated label Sep 15, 2022

openjdk bot closed this Sep 15, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Sep 15, 2022

openjdk bot removed the sponsor Pull request is ready to be sponsored label Sep 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8292948: JEditorPane ignores font-size styles in external linked css-file #10054

8292948: JEditorPane ignores font-size styles in external linked css-file #10054

stanio commented Aug 27, 2022 •

edited by openjdk bot

Loading

bridgekeeper bot commented Aug 27, 2022

openjdk bot commented Aug 27, 2022

mlbridge bot commented Aug 27, 2022 •

edited

Loading

stanio commented Aug 28, 2022

openjdk bot commented Sep 2, 2022 •

edited

Loading

stanio commented Sep 12, 2022

stanio commented Sep 12, 2022

openjdk bot commented Sep 12, 2022

prsadhuk commented Sep 15, 2022

openjdk bot commented Sep 15, 2022

openjdk bot commented Sep 15, 2022

	Object doGetAttribute(Object key) {
	if (key == CSS.Attribute.FONT_SIZE && !isDefined(key)) {
	// CSS.FontSize represents a specified value and we need
	// to inherit a computed value so don't resolve percentage
	// value from parent.
	return fontSizeInherit();

	Object retValue = super.getAttribute(key);
	if (retValue != null) {
	return retValue;

8292948: JEditorPane ignores font-size styles in external linked css-file #10054

8292948: JEditorPane ignores font-size styles in external linked css-file #10054

Conversation

stanio commented Aug 27, 2022 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Aug 27, 2022

openjdk bot commented Aug 27, 2022

mlbridge bot commented Aug 27, 2022 • edited Loading

Webrevs

stanio commented Aug 28, 2022

openjdk bot commented Sep 2, 2022 • edited Loading

stanio commented Sep 12, 2022

stanio commented Sep 12, 2022

openjdk bot commented Sep 12, 2022

prsadhuk commented Sep 15, 2022

openjdk bot commented Sep 15, 2022

openjdk bot commented Sep 15, 2022

stanio commented Aug 27, 2022 •

edited by openjdk bot

Loading

mlbridge bot commented Aug 27, 2022 •

edited

Loading

openjdk bot commented Sep 2, 2022 •

edited

Loading