Closed Bug 385615 Opened 17 years ago Closed 3 days ago

{inc} style of :first-letter used to erroneously calculate (intrinsic) width of element

Categories

(Core :: Layout, defect)

x86
All
defect

Tracking

()

RESOLVED FIXED
129 Branch
Webcompat Priority P2
Tracking Status
firefox-esr115 --- wontfix
firefox126 --- wontfix
firefox127 --- wontfix
firefox128 --- wontfix
firefox129 --- fixed

People

(Reporter: martijn.martijn, Assigned: jfkthame)

References

(Depends on 1 open bug)

Details

(Keywords: parity-chrome, regression, testcase)

Attachments

(11 files, 1 obsolete file)

131 bytes, text/html
Details
656 bytes, application/xhtml+xml
Details
1.28 KB, text/html
Details
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
See testcase, when clicking on the button, it shrinks.
Although the end result looks more correct to me, the button should not change size when clicking on it.
OS: Windows XP → All
Another testcase, on a div this time.  Button sets then removes a padding.
removed invalid styling "none" and added a setTimeout since otherwise it didn't apply the styling.
Attachment #318181 - Attachment is obsolete: true
Also odd, is that in the (fixed) attachment after triggering recalc, there is still some white space to the right inside the div.
Summary: Shrinking button, use of ::first-letter height: 600% → font-size of :first-letter used to erroneously calculate width of element
Summary: font-size of :first-letter used to erroneously calculate width of element → {inc} font-size of :first-letter used to erroneously calculate width of element
Blocks: 442043
Summary: {inc} font-size of :first-letter used to erroneously calculate width of element → {inc} font-size of :first-letter used to erroneously calculate (intrinsic) width of element
Attached some more test-cases.

What I can see: with larger font-size the bigger extra width would be, the more letters after the first, again the bigger the extra width would be.

Adding float to first-letter invert this behavior, but the extra would depend only on the font-size of the first letter.

And if the font-size of the first-letter is lesser than the inline-block's, then the width of this inline-block “shrinks”.
Blocks: 718424
Blocks: 765798
Crosslinking another Stack overflow question: http://stackoverflow.com/questions/14130653/firefox-calculating-blocks-larger/14131771#14131771

A block-level link shrinks (recalculates its width) to the correct size when focused. All (left, right, middle) clicks trigger the fix. Tabbing onto the link triggers the fix if not prevented by Javascript. Some hover effects also fix the bug.

Manifests as an unwanted gap between floating elements.
Attachment #562320 - Attachment mime type: text/plain → text/html
Any chance of getting it fixed soon? This bug has been open for 6 years and the styling errors caused by this bug are very visible and workarounds are so ugly... This bug is a huge PITA.
Still present in FF 38. Not only "font-size" but also "text-transform" triggers the bug. I'm not sure why but in my case "uppercase" makes a way bigger bug-padding than "capitalize", although the result should be (and is in standard-compliant browsers) the same.
This applies to any aspect of the style of ::first-letter that may affect measurement (not only font-size). E.g. it can be reproduced with text-transform, font-family, font-stretch, font-weight, letter-spacing, etc. Updating the summary to make it more general.
Summary: {inc} font-size of :first-letter used to erroneously calculate (intrinsic) width of element → {inc} style of :first-letter used to erroneously calculate (intrinsic) width of element
Blocks: 1311298
No longer blocks: 1311298
Note that bug 362880 comment 2 has some thoughts on how to fix this.
Webcompat Priority: --- → ?
Webcompat Priority: ? → P2
Severity: normal → S3

The severity field for this bug is relatively low, S3. However, the bug has 24 duplicates and 14 votes.
:dholbert, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Flags: needinfo?(dholbert)

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.

Flags: needinfo?(dholbert)
See Also: → 1834521
Duplicate of this bug: 1834521
See Also: 1834521

I just ran in to this same issue: CodePen example. https://codepen.io/rickdoesburg/pen/KKbjKpB

Bit of an update. I ran into what appears to be the same behaviour, and came up with a workaround.
https://m8y.org/tmp/testcase502.xhtml

The desired behaviour was that the elements would be the width of the first letter and no more. What was happening was that they were wider than this. -But, by an amount that did not seem to be related to the length of the overall content, or the width of the first letter, or the default styling of the span-. CORRECTION. I had tested wrong element when I added extra content (the fix one). It is indeed using first-letter styling for all letters, confirmed by adding a lot more text to the 2nd span. Sorry. So it is indeed this bug...

However! I did verify that this is "fixed" by adding a CSS animation :D :D :D
So. Since this is the same issue as people are experiencing here, there is a (silly) workaround for Firefox you can drop into your stylesheet.
Checkout the max-width test in the 2nd portion. I'm just adding an animation that shrinks the max-width by 1% for 0.01s, no repeat. This change which seems to get ignored by everyone else since it shouldn't impact the layout, causes a recalculation in Firefox.

None of the real-world sites linked in the see-also's and in the see-alos's of dependencies/duplicates of this bug still exist or show an issue. So all we have is isolated test-cases - unsetting the webcompat-priority flag per our rules.

Webcompat Priority: P2 → ---

(In reply to Dennis Schubert [:denschub] from comment #42)

None of the real-world sites linked in the see-also's and in the see-alos's of dependencies/duplicates of this bug still exist or show an issue. So all we have is isolated test-cases - unsetting the webcompat-priority flag per our rules.

I created several sites affected by this bug during the COVID-19 pandemic for various small businesses and nonprofits. Several of those sites are still out there. Here are a couple of examples:

https://websitetemplateproject.org/

https://oakparkmuslims.org/

Thanks for the nudge! I don't see an obvious difference in how those sites render in a quick test. But given your message (and a quick chat with dholbert just now), I'll restore the webcompat-priority to a P3. If you have examples where this is visually affecting the layout, I do encourage you to file a new bug in the Web Compatibility::Site Reports component, that would make it much easier for us to track! :)

Webcompat Priority: --- → P3

Given the regularity with which we get dupes reported here, I think this deserves to be higher than a webcompat-P3.... yes, sites from old reports may no longer exist (or have changed to avoid the bug), but authors keep running into this, realizing it's a Firefox bug, and filing new bugs with new testcases based on whatever they were trying to do on their site.

I count at least 40-plus separate reports across the years, all stemming from this same root issue, which I think indicates that it's causing ongoing difficulties for web developers.

Webcompat Priority: P3 → revisit

I think revisit is the wrong tag tho, that's "let's revisit it at some point in the future", I believe :)

Webcompat Priority: revisit → ?

So... I agree with jfkthame... I run into this periodically, realise it's the same issue I've worked around before, and work around it again by slapping an extra element in or doing something else (like that css animation). That doesn't mean it isn't annoying.
And, yeah, the workaround I gave in comment #41 is a testcase, but I can't link you to the actual site since it's an internal site.
If I ever use that workaround on a public site, I'll be sure to link it.

Thanks for flagging this again! It's right, we do see a lot of duplicates, and it can be a significant visual difference. P2 seems right. We'll import the existing webcompat-reports to Bugzilla soon so we can properly score this.

Webcompat Priority: ? → P2
Keywords: parity-chrome

Verified this issue and could not reproduce it on Firefox Nightly. Since the issue is addressing an internal site to which we do not have access, and with the given test case I could not reproduce, I will leave this issue opened.

Tested with:

Browser / Version: Firefox Nightly 128.0a1 (2024-05-21) (64-bit)
Operating System: Windows 10 PRO x64

(In reply to Raul Bucata from comment #49)

Verified this issue and could not reproduce it on Firefox Nightly.

This does reproduce in Nightly, e.g. with the testcases attached here.

E.g. load https://bugzilla.mozilla.org/attachment.cgi?id=269562. Observe that the button has quite a lot of padding each side. Now press Ctrl-+ to zoom the page, and then Ctrl-0 to reset the zoom level. Observe how the button size unexpectedly changes when the page is zoomed. This is because it was incorrect after the initial reflow.

Similarly, load https://bugzilla.mozilla.org/attachment.cgi?id=562320. Observe how the shaded backgrounds don't match the width of the text in the various elements. Again, zoom the page to force a reflow, and see how it corrects itself.

I just reran my testcase in comment #41 (click on the link - it's a public demo) and verified that the bug still occurs in latest nightly as of 2024-05-22
I can also confirm that my workaround in that same testcase still works, if people want to use that (it's a simple drop-in CSS hack).

Important note with regards to comment #49 - if it seems "Fixed" in nightly, try refreshing the page. The rendering bug seems inconsistent and breaks 95% of the time but not, you know, 100%. I don't know what the reasons for that are. Caching? Something triggering a similar recalculation to my CSS hack? Verified the bug still is active in nightly on Linux and Windows 10.

Attachment #269562 - Attachment description: testcase → testcase 1 (bug: button has extra padding which disappears when you zoom in/out)

See testcase, when clicking on the button, it shrinks

I assume Raul's comment 49 was going off of these^ instructions from comment 0. For me too, this part doesn't reproduce anymore (the button doesn't change size for me when clicked), but that's probably just because we've gotten smarter about avoiding unnecessary reflow when the button is clicked in this case. The fact remains that we start out with the button too large, and that's the bug here; and the button does still unexpectedly shrink if you use full-page zoom (which does force it to reflow). I added a note to the attachment description to make that clearer.

Ah. Gotcha. Good to know.
For my part, I had just switched back to the Windows 10 VM at work where I was rechecking testcase502.xhtml and was shocked that the first red line was same width as the 2nd, so I added a "not 100%" addendum. I have no idea what caused that to happen though. Maybe due to my updating the nightly? Reconnecting to VM/window resize? In any case the bug reappeared when I refreshed.

Indeed, zooming in and out reproduces the bug. Also, refreshing the page without doing any zooming on the page shows a different button size compared to Chrome.

I was confused by the original STRs: "See testcase, when clicking on the button, it shrinks", which is usually our to-go STRs. But will take that into consideration for future references. Thanks for the heads-up.

No behavior change here, just factoring out a helper that will be wanted
for the following patch.

Assignee: nobody → jfkthame
Status: NEW → ASSIGNED

If we're querying the intrinsic widths of a first-letter and its continuation
before they have been reflowed, we need to check the extent of the first-letter
text (similarly to what nsTextFrame::ReflowText does) to avoid measuring too
much of the content using the first-letter styling.

This patch checks the first-letter length during intrinsic size computation,
but does not actually work in most cases because nsTextFrame::SetLength will
bail out if there is not already a next-in-flow frame. The following patch
will address that.

This is an alternative version of SetLength to be used for the child of an nsFirstLetterFrame,
when we may need to create continuations in order to separate the first-letter from its
following text.

With this, the intrinsic sizes of first-letter should be computed correctly.

Also add an extra first-letter-width testcase (based on a case that was still failing
with an earlier iteration of the fix).

Pushed by jkew@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5cd51f055a1a
patch 1 - Factor out a GetContentNewLineOffset() helper in nsTextFrame. r=layout-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/82242b2d942d
patch 2 - Determine extent of ::first-letter range during intrinsic-width computation. r=layout-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/30718580670d
patch 3 - Implement nsTextFrame::SetFirstLetterLength to constrain first-letter during intrinsic-size calculation. r=layout-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/36fc4e06713d
patch 4 - Remove failure annotation for first-letter-width test that now passes. r=layout-reviewers,emilio
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/46507 for changes under testing/web-platform/tests
Pushed by acseh@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f6022837406b
followup, adjust assertion count for crashtest 478185-1.html.
Pushed by smolnar@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/07004e2608f3
Update wpt test @ testing/web-platform/meta/css/CSS2/selectors/first-letter-punctuation/<...>
Pushed by smolnar@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fe82d67b63ef
Further update wpt test @ testing/web-platform/meta/css/CSS2/selectors/first-letter-punctuation/<...> CLOSED TREE
Pushed by smolnar@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f9c2e5707d43
Remove testing/web-platform/meta/css/CSS2/selectors/first-letter-punctuation-068.xht.ini CLOSED TREE
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/aec1be189f68
skip test 478185-1 on Windows 32-bit debug because it can be too slow to load. CLOSED TREE
Duplicate of this bug: 362880
Upstream PR merged by moz-wptsync-bot
Regressions: 1900169
Regressions: 1899840
Pushed by jkew@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f10b8ba44d94
Backed out 9 changesets (aec1be189f68, f9c2e5707d43, fe82d67b63ef, 07004e2608f3, f6022837406b, 36fc4e06713d, 30718580670d, 82242b2d942d, 5cd51f055a1a) due to reported regressions.
Pushed by abutkovits@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/709248f1fc69
Backed out 9 changesets (aec1be189f68, f9c2e5707d43, fe82d67b63ef, 07004e2608f3, f6022837406b, 36fc4e06713d, 30718580670d, 82242b2d942d, 5cd51f055a1a) due to reported regressions.
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/46595 for changes under testing/web-platform/tests
Upstream PR merged by moz-wptsync-bot
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: 128 Branch → ---
Status: REOPENED → RESOLVED
Closed: 16 days ago10 days ago
Resolution: --- → FIXED
Target Milestone: --- → 128 Branch

I think this was closed accidentally.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: 128 Branch → ---

This reverts the backout, effectively re-landing all the original patches here and the test-manifest
followups that were pushed after the original landing.

This fixes the fuzzer-found assertion reported in bug 1899840, as well as the real-world
website hangs reported in bug 1900169.

In addition, it adds a pref (layout.css.intrinsic-size-first-letter.enabled) that gates
the new functionality during intrinsic size computation. This gives us a way to easily
disable it in the event of other regressions showing up.

Also add the testcase from 1899840 as a wpt crashtest.

Pushed by jkew@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d8ab045a10e2
Revert backout changeset 709248f1fc69, to re-land the original patch stack. (no review)
https://hg.mozilla.org/integration/autoland/rev/e07ecd4899ee
Avoid creating zero-length textframes when trying to handle ::first-letter. r=dholbert
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/46679 for changes under testing/web-platform/tests
Status: REOPENED → RESOLVED
Closed: 10 days ago3 days ago
Resolution: --- → FIXED
Target Milestone: --- → 129 Branch
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: