Closed
Bug 1306590
Opened 7 years ago
Closed 3 years ago
percent "text-indent" values resolve against the wrong container box, for overflow:hidden elements (causing stray character to be visible at reddit)
Categories
(Core :: Layout: Text and Fonts, defect, P3)
Core
Layout: Text and Fonts
Tracking
()
People
(Reporter: JuliaC, Unassigned)
References
Details
(Whiteboard: [webcompat])
Attachments
(5 files, 1 obsolete file)
[Affected versions]: - latest Nightly 52.0a1 (2016-09-29) - latest Aurora 51.0a2 (2016-09-29) - 50.0b3 build1 (20160929120120) - 49.0.1 build3 20160922113459 [Affected platforms]: - Windows 10 x86 - Windows 7 x64 - Mac OS X 10.10.5 - Ubuntu 14.04 x64 [Steps to reproduce]: 1. Launch Firefox 2. Go to https://www.reddit.com/ 3. Choose a random post and click the "share" button - inspect the Share window [Expected result]: - The Share window is properly open and no visual issues come out [Actual result]: - Some visual issues can be notice in the Share window, near the "X" button (see the attached screenshot) [Regression range]: - I will investigate this as soon as possible
![]() |
||
Comment 1•7 years ago
|
||
![]() |
||
Comment 2•7 years ago
|
||
At least, I can reproduce the problem on very old Firefox4.0.
Component: Graphics → Layout
Keywords: regression,
regressionwindow-wanted
Comment 3•7 years ago
|
||
The issue seems to be with "text-indent: 100%". In Chrome, that pushes the text ("close this window") entirely outside of the container. In Firefox, it does not (quite).
Component: Layout → Layout: Text
Comment hidden (obsolete) |
Comment 5•7 years ago
|
||
Comment on attachment 8796639 [details]
(ignore, better testcase coming up next)
(ah, disregard previous comment/testcase; another better version coming up shortly)
Attachment #8796639 -
Attachment description: testcase 2 → (ignore, better testcase coming up next)
Attachment #8796639 -
Attachment is obsolete: true
Comment 6•7 years ago
|
||
Comment 7•7 years ago
|
||
Comment 8•7 years ago
|
||
So per https://developer.mozilla.org/en-US/docs/Web/CSS/text-indent , "percentages refer to the width of the containing block". In Firefox, when we have "overflow:hidden", the containing block is the scrollable frame (the overflow:hidden thing). In Chrome, they're apparently interpreting "containing block" to mean the *container* of the scrollable frame. I'm not sure offhand who's correct... BUT: - given that "overflow:hidden" creates two nested boxes (the outer one with the specified size & the inner one that gets clipped), I tend to think it makes sense that we consider the outer box the containing block of the inner one. - Reddit's "100%" here seems like it likely wants to refer to "100% of the width of this box". In Firefox, it does (though there's padding which is why there's a glitch). In Chrome, it refers to 100% of the width of some ancestor box which is probably way huger. So while we sort out whether or not this is a Firefox bug (and in the fastest case, ship a fix which will take a few months to make it to release), I think Reddit probably really wants to either: (a) change their .c-hide-text CSS rule to use "text-indent: 200%" (instead of 100%), for good measure (which will work as long as the extra padding isn't larger than the content-box size) ...and/or possibly: (b) use "color: transparent" to make the text invisible, if it does happen to show up. (I'm assuming they have it there for screen-reader purposes, and hopefully screen readers will read transparent text, though I'm not entirely sure.) Adam, could you reach out to someone at Reddit and suggest that they make change (a) here?
Comment 9•7 years ago
|
||
[er, meant to direct end of comment 8 at @astevenson]
Flags: needinfo?(astevenson)
Comment 10•7 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #6) > Created attachment 8796651 [details] > testcase 2 It looks like Edge renders this the same as Chrome and Safari.
Comment 11•7 years ago
|
||
Opera 12.6 (Presto) renders testcase 2 like Chrome, too -- so it looks like we indeed are the odd one out here.
Summary: Visual issues on https://www.reddit.com/ → percent "text-indent" values resolve against the wrong box, for overflow:hidden elements (causing stray character to be visible at reddit)
Updated•7 years ago
|
Summary: percent "text-indent" values resolve against the wrong box, for overflow:hidden elements (causing stray character to be visible at reddit) → percent "text-indent" values resolve against the wrong container box, for overflow:hidden elements (causing stray character to be visible at reddit)
Comment 12•7 years ago
|
||
This shouldn't be too hard to fix. Wherever we resolve this percentage, we need to just add a special-case for scrolled frames, to step over the outer scroll frame and look at *its* containing block as our percent-basis, when we resolve percent values for this property during layout. SPECIFICALLY: I think the relevant place where we resolve the percentage is here: https://dxr.mozilla.org/mozilla-central/rev/9baec74b3db1bf005c66ae2f50bafbdb02c3be38/layout/generic/nsLineLayout.cpp#223-229 ...though we should audit other usages of mTextIndent, too, and see if any of them attempt to resolve the percent value. And to test whether we're a scrolled frame there, I think we'd check whether mBlockReflowInput->mFrame->StyleContext()->GetPseudo() == nsCSSAnonBoxes::scrolledContent. And if that's true, we'd want to look outside of that frame for the containing block. (get its parent's containing block, or something like that)
Comment 13•7 years ago
|
||
(Actually, it might be cleaner to fix this by changing the internals of the ReflowInput::GetContainingBlockContentISize(), so that it recognizes whether we're scrolled-content and steps out appropriately... This is the only caller of that method, so that wouldn't break assumptions in other code.) One open question here, which we should figure out before/while fixing this: - How/why do we get this correct for percent "padding-left" values? Both "padding-left" and "text-indent" seem to be resolved using information from ReflowInput::mCBReflowInput. But for "padding-left", we do seem to resolve the percentages against the outer containing block, rather than against the size of the scroll-frame. Whatever functionality we use to do that, we should probably share or mimic for this text-indent resolution. (BTW, percent "padding" values get resolved in ReflowInput::InitOffsets, below here: https://dxr.mozilla.org/mozilla-central/rev/9baec74b3db1bf005c66ae2f50bafbdb02c3be38/layout/generic/ReflowInput.cpp#2445
Comment 14•7 years ago
|
||
Comment 15•7 years ago
|
||
Daniel: I think this is what we are looking for. https://github.com/reddit/reddit/blob/862a7e0fc5142058a88044f9254a8ec5db977a77/r2/r2/public/static/css/components/utils.less#L17
Flags: needinfo?(astevenson)
Comment 16•7 years ago
|
||
Nice sleuthing! I think you're right. I think if they replace "100%" with "200%" there, that should avoid the issue in Firefox (without affecting the rendering anywhere else). If you file a github issue, maybe link it from here? Meanwhile, I think Jet is going to find a rendering-team engineer to work on this.
Updated•7 years ago
|
Flags: needinfo?(bugs)
Comment 17•7 years ago
|
||
Reported: https://github.com/reddit/reddit/issues/1700
Updated•7 years ago
|
Whiteboard: [webcompat]
Updated•7 years ago
|
Priority: -- → P3
Comment 18•6 years ago
|
||
This sounds like a duplicate of bug 908706.
Comment 19•6 years ago
|
||
Bug 1453298 makes this invalid.
Comment 21•5 years ago
|
||
Migrating Webcompat whiteboard priorities to project flags. See bug 1547409.
Webcompat Priority: --- → ?
Comment 22•5 years ago
|
||
See bug 1547409. Migrating whiteboard priority tags to program flags.
Comment 23•4 years ago
|
||
(In reply to David Baron :dbaron: 🏴 ⌚UTC-7 from comment #19)
Bug 1453298 makes this invalid.
reddit.com was redesigned, so the reported issue no longer exists. Unsure if we should just close this as INVALID, given dbaron's comment?
Webcompat Priority: ? → ---
Comment 24•3 years ago
|
||
Close per comment 19.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•