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

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase 1 (bug: button has extra padding which disappears when you zoom in/out)

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.

Comment 1

[nemo]

Attachment: testcase with a floated block with first-letter

Another testcase, on a div this time.  Button sets then removes a padding.

Comment 2

[nemo]

Attachment: fix of javascript in prior attachment

removed invalid styling "none" and added a setTimeout since otherwise it didn't apply the styling.

Comment 3

[nemo]

Also odd, is that in the (fixed) attachment after triggering recalc, there is still some white space to the right inside the div.

Comment 8

[RNicoletto]

Possible workaround: https://addons.mozilla.org/firefox/addon/bug385615/

Comment 11

[Roman Komarov]

Attachment: Some more test-cases with different font-sizes and floating first-letters

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”.

Comment 13

[Elijah Lynn]

Want to cross link two in the wild bug reports to this issue.

http://stackoverflow.com/questions/13160161/firefox-whitespace-oddity-bug-on-a-element/

http://stackoverflow.com/questions/7548255/is-there-a-css-workaround-for-firefox-bug-inline-block-first-letter-with-cha

Comment 15

[jan dvorak]

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.

Comment 18

[Michał Gołębiowski-Owczarek [:mgol]]

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.

Comment 20

[m93a.cz]

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.

Comment 23

[Jonathan Kew [:jfkthame]]

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.

Comment 28

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Note that bug 362880 comment 2 has some thoughts on how to fix this.

Comment 33

[Alice0775 White]

(sorry, maybe off-topic)
Interestingly, If I installed Glitter Drag[1] extension, the problem is gone.
[1]https://addons.mozilla.org/en-US/firefox/addon/glitterdrag/

Comment 37

[BugBot [:suhaib / :marco/ :calixte]]

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.

Comment 38

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

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.

Comment 40

[rickdoesburg]

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

Comment 41

[nemo]

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.

Comment 42

[Dennis Schubert [:denschub]]

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.

Comment 43

[Mohammed Abouzahr]

(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/

Comment 44

[Dennis Schubert [:denschub]]

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! :)

Comment 45

[Jonathan Kew [:jfkthame]]

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.

Comment 46

[Emilio Cobos Álvarez (:emilio)]

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

Comment 47

[nemo]

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.

Comment 48

[Dennis Schubert [:denschub]]

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.

Comment 49

[Raul Bucata]

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

Comment 50

[Jonathan Kew [:jfkthame]]

(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.

Comment 51

[nemo]

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).

Comment 52

[nemo]

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.

Comment 53

[Daniel Holbert [:dholbert]]

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.

Comment 54

[nemo]

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.

Comment 55

[Raul Bucata]

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.

Comment 56

[Jonathan Kew [:jfkthame]]

Attachment: Bug 385615 - patch 1 - Factor out a GetContentNewLineOffset() helper in nsTextFrame. r=#layout-reviewers

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

Comment 57

[Jonathan Kew [:jfkthame]]

Attachment: Bug 385615 - patch 2 - Determine extent of ::first-letter range during intrinsic-width computation. r=#layout-reviewers

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.

Comment 58

[Jonathan Kew [:jfkthame]]

Attachment: Bug 385615 - patch 3 - Implement nsTextFrame::SetFirstLetterLength to constrain first-letter during intrinsic-size calculation. r=#layout-reviewers

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.

Comment 59

[Jonathan Kew [:jfkthame]]

Attachment: Bug 385615 - patch 4 - Remove failure annotation for first-letter-width test that now passes. r=#layout-reviewers

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

Comment 60

[Pulsebot]

Pushed by [e-mail]:
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

Comment 61

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/46507 for changes under testing/web-platform/tests

Comment 62

[Jonathan Kew [:jfkthame]]

Attachment: Bug 385615 followup, adjust assertion count for crashtest 478185-1.html.

Comment 63

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f6022837406b
followup, adjust assertion count for crashtest 478185-1.html.

Comment 64

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/07004e2608f3
Update wpt test @ testing/web-platform/meta/css/CSS2/selectors/first-letter-punctuation/<...>

Comment 65

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fe82d67b63ef
Further update wpt test @ testing/web-platform/meta/css/CSS2/selectors/first-letter-punctuation/<...> CLOSED TREE

Comment 66

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f9c2e5707d43
Remove testing/web-platform/meta/css/CSS2/selectors/first-letter-punctuation-068.xht.ini CLOSED TREE

Comment 67

[Pulsebot]

Pushed by [e-mail]:
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

Comment 68

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/5cd51f055a1a
https://hg.mozilla.org/mozilla-central/rev/82242b2d942d
https://hg.mozilla.org/mozilla-central/rev/30718580670d
https://hg.mozilla.org/mozilla-central/rev/36fc4e06713d
https://hg.mozilla.org/mozilla-central/rev/f6022837406b
https://hg.mozilla.org/mozilla-central/rev/07004e2608f3
https://hg.mozilla.org/mozilla-central/rev/fe82d67b63ef
https://hg.mozilla.org/mozilla-central/rev/f9c2e5707d43
https://hg.mozilla.org/mozilla-central/rev/aec1be189f68

Comment 70

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 71

[Jonathan Kew [:jfkthame]]

Attachment: Bug 385615 - Backed out 9 changesets (aec1be189f68, f9c2e5707d43, fe82d67b63ef, 07004e2608f3, f6022837406b, 36fc4e06713d, 30718580670d, 82242b2d942d, 5cd51f055a1a) due to reported regressions.

Comment 72

[Pulsebot]

Pushed by [e-mail]:
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.

Comment 73

[Pulsebot]

Pushed by [e-mail]:
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.

Comment 74

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/46595 for changes under testing/web-platform/tests

Comment 75

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/709248f1fc69

Comment 76

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 77

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/f10b8ba44d94

Comment 78

[Timothy Nikkel (:tnikkel)]

I think this was closed accidentally.

Comment 79

[Jonathan Kew [:jfkthame]]

Attachment: Bug 385615 - Revert backout changeset 709248f1fc69, to re-land the original patch stack. (no review)

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

Comment 80

[Jonathan Kew [:jfkthame]]

Attachment: Bug 385615 - Avoid creating zero-length textframes when trying to handle ::first-letter. r=#layout

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.

Comment 81

[Pulsebot]

Pushed by [e-mail]:
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

Comment 82

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/46679 for changes under testing/web-platform/tests

Comment 83

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/d8ab045a10e2
https://hg.mozilla.org/mozilla-central/rev/e07ecd4899ee

Comment 84

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot