290125 - [CSS21] floated :first-letter pseudo-element should act like normal inline (e.g., support line-height)

Status: RESOLVED FIXED

(Core :: Layout: Floats, defect, P3)

Description

[philippe (part-time)]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050412 Firefox/1.0+ (PowerBook)
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050412 Firefox/1.0+ (PowerBook)

When floating left a :first-letter (to produce a dropcap), Gecko ignores any
declared line-height and inherits the line-height of the parent box. This
prevent authors from positioning the first letter by reducing the line-box
through the use of a small line-height. Wether the leni-height on the
:first-letter is 0.7em or 1.5em, the line box remains the same.
(both Opera 7.5+ and Safari 1.0+ correctly handlle this)

Reproducible: Always

Actual Results:  
Line-height declaration ignored.

Expected Results:  
Respecting the line-height rule.

Comment 1

[philippe (part-time)]

Attachment: test case

Test case showing the problem. 3 groups of paragraphs; each groups has a
first-letter with different line-height.

Comment 2

[philippe (part-time)]

While applying a workaround for this bug on a live site, I noticed that the
presence of numerical entities in the text does affect the height of the
line-box for the :first-letter. The line-height for the :first-letter is smaller
for paragraphs of text that contain numerical entities.
Without the entities, the line-height of the :first-letter is inherited from the
parent block, and then computed based on the font-size of the :first-letter.
With the numerical entities,  the line-height of the :first-letter is actually
the computed value of the line-height (in pixels) of the parent paragraph.

However, when those entities are wrapped in an inline tag (span, strong, cite,
...), the :first-letter is not affected. Two live example
http://emps.l-c-n.com/articles/84/ the first-letter jumps upwards
http://emps.l-c-n.com/articles/83/ works correctly, the entities are wrapped in
a cite.

I'll attach a simplified test-case

Comment 3

[philippe (part-time)]

Attachment: test case for comment 2

illustrating the problems with numerical entities

Comment 4

[Boris Zbarsky [:bzbarsky]]

This isn't really a floats bug...  It's also not clear how setting line-height
interacts with the weasel-language in the CSS spec about taking glyph outlines
into account.

Ian, how's this supposed to work (and where in the spec is that actually explained)?

Comment 5

[Hixie (not reading bugmail)]

The testcase renders all three cases the same for me.

This should render however is most appropriate, typographically. The CSS spec
explicitly doesn't define ::first-letter to the letter because we want UAs to be
able to come up with better algorithms.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Ian, the question is what effect, if any, setting line-height should have on the
first-letter.

Comment 7

[Hixie (not reading bugmail)]

Up to the UA. Specifically:

# To allow UAs to render a typographically correct drop cap or initial cap, the 
# UA may choose a line-height, width and height based on the shape of the letter,
# unlike for normal elements.

...so if we do shrink-wrapping of letters to get good drop/initial caps, then we
can ignore line-height.

If we don't do that, then instead we must treat line-height as described in the
spec for ordinary spans and floats and so forth.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Ah, ok.  We do exactly such shrink-wrapping.

Comment 9

[philippe (part-time)]

(In reply to comment #7)

From wich I understand that Gecko does not apply line-height to the
:first-letter. Fair enough according to the specs, although different from what
other UA's do (Opera, Safari).

But there is still a problem with this. If you look at the second test case,
where I inserted a numerical entity (’) in the text (in the first
paragraph with a grey border). The linebox of the first letter is much smaller
than when no num. entity is inserted, causing the first-letter to jump upwards.
Or should I file a new bug, specifically for this ?

Comment 10

[Boris Zbarsky [:bzbarsky]]

Possibly.  I can't reproduce the behavior you're describing -- the three sets of
tests in the second testcase look identical as far as first-letter positioning
goes in a current Linux trunk build.

Comment 11

[philippe (part-time)]

(In reply to comment #10)
> Possibly.  I can't reproduce the behavior you're describing -- the three sets of
> tests in the second testcase look identical as far as first-letter positioning
> goes in a current Linux trunk build.

It turns out to be a Mac OS X only problem.

While reviewing my test files, I noticed also that the OS X builds behave
differently from the Win XP builds (tested: 20050517). I'll attach a screenshot
in moment.

Requesting to reopen this, but only for OS X builds.

Comment 12

[philippe (part-time)]

Attachment: testcase with and without num. entities

One more testcase illustrating the problem(s). One character entity use: &
#8217;. See the difference in the size of the first-letter line-box between the
paragraphs without (red border) and with (grey border) num. entities.

Comment 13

[philippe (part-time)]

Attachment: screen shot of the third case

On the left OS X 10.3.9, Firefox 20050517.
On the right XP Firefox 20050517.

Comment 14

[philippe (part-time)]

The shrinked line-box for first-letter doesn't happen when the num.entity is
preceeded by a span (<span>, <strong>,<a>, ...) - third group of paragraphs in
testcase 3.
It doesn't happen with all numerical entity either. Using &#39 instead of
&#8217, and the line-box of the first-letter doesn't shrink.  Any entity above
&#81xx does show the problem. I've also tested inserting a couple of Japanese
characters, this has the same effect as using anything from &#81xx onwards.

Comment 15

[Boris Zbarsky [:bzbarsky]]

If this is mac-only, I suggest filing a new clear bug on GFX:Mac with a pointer
to these testcases...

Comment 16

[philippe (part-time)]

(In reply to comment #15)
> If this is mac-only, I suggest filing a new clear bug on GFX:Mac with a pointer
> to these testcases...

OK, I filed bug 294733

Comment 17

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

Given that:
 (a) Firefox was the only browser implementing this special behavior
 (b) css-inline is bringing better initial letter support
the working group decided to remove the special case for doing floating ::first-letter better, so we should remove the corresponding code.

Comment 20

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

CSSWG resolution in http://lists.w3.org/Archives/Public/www-style/2014Sep/0384.html

Comment 21

[Hallvord R. M. Steen [:hallvors]]

This might be a dupe of bug 13610 or vice versa

Comment 24

[Jeremy Chen ([e-mail])]

Quote David's comment in bug 1233271 (see bug 1233271 comment 3) below, so people who track from other duplicated bugs could have a clear explanation.

==
http://www.w3.org/TR/CSS21/selector.html#first-letter says:
  To allow UAs to render a typographically correct drop cap or initial cap, the UA may choose a line-height, width and height based on the shape of the letter, unlike for normal elements.

This has been what we do for a long time (I think since it was more strongly recommended by the spec).

That said, given that no other implementation does, we should probably stop doing so.
==

Comment 26

[Thomas Wisniewski]

dbaron, do you still think this is something we should fix, and if so, if it would be difficult to fix anytime soon? I wouldn't mind trying my hand at making a patch, if you could give me some pointers on what would need to be changed.

Comment 27

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

I think it's pretty easy to do; it requires removing special-case code.

However, I'm inclined to think we should do it when or after we ship initial-letter support, not before.

Comment 28

[Florens Verschelde :fvsch]

Since late 2016 or early 2017 I’m seeing several news sites which use `line-height:.6` or similar for big first-letters (2-3 lines high). Apparently this design trend is back.

In Firefox the letter appears lower than expected, sometimes overlapping the next line. Developers seem content with this working in Chrome and Safari and looking busted in Firefox.

Some examples:
- T in "This month", overlaps with other text:
  https://www.theverge.com/2017/7/29/16017286/formula-e-new-york-brooklyn-photos-pictures
- I in "Is a $4 million", misaligned (too low) but not overlapping:
  https://theoutline.com/post/1953/how-a-vc-funded-company-is-undermining-the-open-source-community

Counter-example:
- Using a <span> element for compatibility with Fx:
  https://www.theguardian.com/world/2017/jul/30/palantir-peter-thiel-cia-data-crime-police

Fixing this bug without depending on 1223880 (which is unassigned) might be a quick win.

Comment 30

[Karl Dubost💡 :karlcow]

We receive at regular pace Webcompat issues for theverge site.

Comment 31

[Cameron McCormack (:heycam)]

(In reply to David Baron :dbaron: ⌚️UTC-8 from comment #27)
> However, I'm inclined to think we should do it when or after we ship
> initial-letter support, not before.

initial-letter isn't going to ship soon, right?  What's the downside of fixing this before initial-letter ships?

Comment 32

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

I guess we could do it sooner.  (At the time I wrote that I was expecting initial-letter to happen faster.)

It means we'll have no way to do properly-wrapped initial letters, which we have in the current code and will have with initial-letter... but nobody can depend on the current thing since it doesn't work cross-browser.

Comment 33

[Cameron McCormack (:heycam)]

(In reply to David Baron :dbaron: ⌚️UTC+2 (mostly unavailable April 2-20) from comment #32)
> It means we'll have no way to do properly-wrapped initial letters, which we
> have in the current code and will have with initial-letter... but nobody can
> depend on the current thing since it doesn't work cross-browser.

I'm not sure what you mean by "properly-wrapped" here.  Anyway, given the effort to make floated ::first-letter behave more like other browsers is smaller than finishing the initial-letter implementation I think we should do it; sites are already assuming it works as it does in Chrome.

Comment 35

[Firefox Bug Husbandry Bot]

See bug 1547409. Migrating webcompat priority whiteboard tags to project flags.

Comment 37

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

The severity field for this bug is relatively low, S3. However, the bug has 8 duplicates and 9 See Also bugs.
:TYLin, 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

[Jonathan Kew [:jfkthame]]

Given that fully implementing initial-letter will take some time (and doesn't help with existing content, as it will need to be adopted by authors), I think we should do something to mitigate the webcompat pain here.

To minimize the risk of regressing existing content that does work well in Firefox, I propose taking a fairly conservative approach here:

(1) Although I think first-letter could benefit from a major refactoring, to move more work to frame-construction time instead of reflow time, let's not do that here; keep the existing code structure and make only minimal changes to achieve a webkit/blink-compatible result;

(2) Implement modified behavior behind a flag, so that we can roll it out initially just to Nightly, and can easily revert to existing behavior in case of problems.

Furthermore, we can probably distinguish between content that works well with our current behavior (typically this would be a "naive" use of first-letter with float:left, and no special effort to adjust the margins around the floated letter) vs content that is authored specifically for the webkit/blink model. In the latter case, the author typically needs to remove excess blank space around the floated letter. I've seen two techniques used for this: setting an unusually small line-height (less than 1em) on the first-letter, so as to reduce the height of its box, or applying negative top and bottom margins to it.

Given this, I think we can usefully implement an "auto" behavior toggle that uses heuristics to choose between our existing "tightly wrapped" behavior and the "loose wrap" model, based on the presence of such line-height or margin values.

Comment 41

[Jonathan Kew [:jfkthame]]

https://codepen.io/jfkthame/pen/qBKeBWd has some examples to compare our behavior vs webkit/blink's. Case (1) there shows how our current "tight wrap" behavior gives a better result when an author simply uses float:left on first-letter, without specifically compensating for blank space around the glyph within the font's overall metrics.

Cases (2) and (3) are examples of how authors often handle this, using either negative margins or reduced line-height. Case (2) looks bad in current Firefox, and is typical of the sort of bug reports we get. Case (3) renders the same as (1) for us, as we ignore the line-height property.

The cases where the first-letter glyph has a descender ("y" or "J") show that the webkit/blink behavior doesn't automatically account for this (as gecko does); the author would need to adjust the margin or line-height properties to avoid a clash in these cases. In practice, though, first-letter is almost always applied to descender-less capitals, and so this doesn't often arise.

My proposed change here to use a heuristic to switch between the two behaviors will keep our existing behavior for case (1), while switching to a webkit/blink-compatible result for (2) and (3).

Comment 42

[Jonathan Kew [:jfkthame]]

Attachment: Bug 290125 - Adjust first-letter reftests to be robust against change to floating-firstletter metrics. r=#layout-reviewers

And fix the overflow-float-overflow.html test to do a != comparison, as indicated by the
-notref naming of the reference file.

No behavior change; this is just adjusting a couple of old reftests so they will not be
affected by the exact detail of how we wrap around a floating first-letter (which is
incidental to what they were aiming to test).

Comment 43

[Jonathan Kew [:jfkthame]]

Attachment: Bug 290125 - Create a pref to treat floated ::first-letter more like webkit/blink, not tightly wrapping the glyph extents. r=#layout-reviewers

Historically, Gecko implemented the behavior allowed by CSS2 whereby a floated ::first-letter is "boxed"
tightly around the glyph shape, rather than using constant font-ascent and -descent metrics which may
leave a lot of blank space depending whether the character has any ascender/descender or not.

However, neither webkit nor blink do this, which leads to webcompat pain when sites are constructed
assuming their behavior.

Eventually, I think we should ideally reimplement ::first-letter entirely at frame-construction time,
rather than during reflow. But in the interest of minimizing risk here, and making it easy to flip
between our existing "legacy" behavior and the new "compatible" behavior, this patch leaves the
overall implementation unchanged and just alters the metrics used for the resulting first-letter
frame.

This patch creates an integer pref layout.css.floating-first-letter.tight-glyph-bounds to allow us
to choose between three behaviors:

1: Use tight glyph bounds, and ignore line-height; the baseline of the floated letter automatically
adjusts to wrap text around the "ink box" of the glyph. This is the existing Gecko behavior.

0: Don't use tight glyph bounds, respect line-height: the floated letter acts like a normal <span>
with float positioning; baseline position and vertical size are based on font metrics but not
the specific shape of the individual glyph. This gives a similar result to webkit/blink.

-1: Automatically choose between (1) and (0) based on heuristics to try and detect whether the page
was written with the webkit/blink behavior (0) in mind; specifically, if there is a line-height
of less than 1em, or a negative block-start margin, we assume the author was trying to eliminate
excess blank space that behavior (0) tends to produce, and so we use that model.

Initially, this patch leaves the behavior unchanged for Beta/Release builds, but enables option -1 (use
heuristics to choose which layout model to apply) on Nightly so we can see how that works in practice.

Depends on D165007

Comment 44

[Jonathan Kew [:jfkthame]]

Attachment: Bug 290125 - Set the floating-firstletter pref to "compatibility" (0) value for some WPT tests that we can now pass. r=#layout-reviewers

Depends on D165008

Comment 45

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3f341b8efb86
Adjust first-letter reftests to be robust against change to floating-firstletter metrics. r=emilio
https://hg.mozilla.org/integration/autoland/rev/d8297eee88e7
Create a pref to treat floated ::first-letter more like webkit/blink, not tightly wrapping the glyph extents. r=emilio
https://hg.mozilla.org/integration/autoland/rev/0b6b18ea2634
Set the floating-firstletter pref to "compatibility" (0) value for some WPT tests that we can now pass. r=emilio

Comment 46

[Cristian Tuns]

Backed out for causing build bustages on nsFirstLetterFrame.cpp

• Backout link
• Push with failures
• Failure Log
• Failure line: /builds/worker/checkouts/gecko/layout/generic/nsFirstLetterFrame.cpp:160:41: error: member access into incomplete type 'nsTextFrame'

and also mochitest failures: https://treeherder.mozilla.org/logviewer?job_id=400126353&repo=autoland

Comment 47

[Serban Stanca [:SerbanS]]

It also fails on Windows 10 x64 2004 reftests with gfxFont::GetMetrics(nsFontMetrics::FontOrientation crash signature.

• Push with failures
• Failure line: REFTEST PROCESS-CRASH | layout/base/crashtests/429865-1.html | application crashed [@ gfxFont::GetMetrics(nsFontMetrics::FontOrientation)]

Comment 48

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7a17a4c4cd6e
Adjust first-letter reftests to be robust against change to floating-firstletter metrics. r=emilio
https://hg.mozilla.org/integration/autoland/rev/02a4e815c82c
Create a pref to treat floated ::first-letter more like webkit/blink, not tightly wrapping the glyph extents. r=emilio
https://hg.mozilla.org/integration/autoland/rev/b476e406831d
Set the floating-firstletter pref to "compatibility" (0) value for some WPT tests that we can now pass. r=emilio

Comment 49

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/7a17a4c4cd6e
https://hg.mozilla.org/mozilla-central/rev/02a4e815c82c
https://hg.mozilla.org/mozilla-central/rev/b476e406831d