419285 - ::first-letter applies to entire line following <br> at start of block

Status: VERIFIED FIXED

(Core :: Layout: Block and Inline, defect)

Description

[Márton Németh]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; hu; rv:1.9b3) Gecko/2008020513 Firefox/3.0b3
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; hu; rv:1.9b3) Gecko/2008020513 Firefox/3.0b3

The sentence "This line should be green." is yellow and has red background at test case http://www.w3.org/Style/CSS/Test/CSS3/Selectors/current/xhtml/full/flat/css3-modsel-180a.xml

Reproducible: Always

Steps to Reproduce:
1. Open test case at http://www.w3.org/Style/CSS/Test/CSS3/Selectors/current/xhtml/full/flat/css3-modsel-180a.xml

Actual Results:  
The sentence "This line should be green." is yellow and has red background.

If the window is resized so that the sentence wrap to multiple lines, then the second line will be green. Then, if the window is resized again so large that the senence would fit into one line, the second line remains.

Expected Results:  
The sentence "This line should be green." is green.

If the window resized, the sentence wraps and unwraps as needed.

Comment 1

[Márton Németh]

Attachment: screenshot with red background on Linux

The screenshot was created with:
Mozilla/5.0 (X11; U; Linux i686; hu; rv:1.9b3) Gecko/2008020513 Firefox/3.0b3

Comment 2

[Márton Németh]

Attachment: window resized to smaller size

Comment 3

[Márton Németh]

Attachment: window restored to original size

The sentence "This line should be green." is still displayed in two lines like this:

This
line should be green.

Comment 4

[philippe (part-time)]

Ouch.
And that is a regression from Gecko 1.8.1

Further more, the :first-letter should not be applied at all, per
http://www.w3.org/TR/css3-selectors/#first-letter

(Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a1pre) Gecko/2008062020 Minefield/3.1a1pre)

Comment 5

[philippe (part-time)]

Attachment: testcase

Comment 6

[philippe (part-time)]

regressed between 2007081504 and 2007081604

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-08-15+04%3A00%3A00&maxdate=2007-08-16+03%3A00%3A00&cvsroot=%2Fcvsroot

--> almost certainly 385270 (caused a similar, but fixed, bug 392435) 

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

nsCSSFrameConstructor::WrapFramesInFirstLetterFrame treats the BR frame as inline since it's an eLineParticipant, so it carries on and wraps the following text frame in an nsFirstLetterFrame. Then during reflow, firstLetterStyleOK is correctly returning false during reflow of the text line --- since it's not the first line --- but nsFirstLetterFrame still has the first-letter style. It only turns off the first-letter style when it creates a continuation and here, since firstLetterStyleOK is false, the text frame doesn't break internally so nsFirstLetterFrame never gets a chance to create a continuation without the first-letter style.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

This approach is very simple and fixes the bug. I think it's the best approach; modifying nsFirstLetterFrame to handle cases where the first-in-flow should not have first-letter style seems hard.

Comment 9

[Zack Weinberg (:zwol)]

I wonder if this is also causing bug 431341.  Will experiment.

Comment 10

[Zack Weinberg (:zwol)]

alas, while I do still suspect the interpolation of nsFirstLetterFrame is causing bug 431341, this patch does not cure it.

Comment 11

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

Comment on attachment 326395 [details] [diff] [review]
fix

r+sr=dbaron

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Pushed a50ca180ed3e

Comment 13

[philippe (part-time)]

Works perfectly with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a2pre) Gecko/2008073016 Minefield/3.1a2pre

Based on comment 8, this sounds a simple fix. Can this be ported to the 1.9.0.x branch ?

Comment 14

[Samuel Sidler (old account; do not CC)]

Yeah, we should get this on 1.9.0.x. roc, can you request approval on a 1.9 patch?

Comment 15

[Samuel Sidler (old account; do not CC)]

Comment on attachment 326395 [details] [diff] [review]
fix

Approved for 1.9.0.2. Please land in CVS. a=ss

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Checked into 1.9.0.

Comment 17

[Al Billings [:abillings - ex-MoCo]]

Verified for 1.9.0.2 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.2) Gecko/2008090212 Firefox/3.0.2.

Comment 18

[Aakash Desai [:aakashd]]

verified FIXED on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090413 Minefield/3.6a1pre ID:20090413031052

and

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090413 Minefield/3.6a1pre ID:20090413031052

Comment 19

[Aakash Desai [:aakashd]]

ack, here's the build ID for Shiretoko Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090413 Shiretoko/3.5b4pre ID:20090413031313