Bug 104485

Summary: Flexbox should ignore firstLine pseudo element.
Product: WebKit Reporter: huangxueqing <huangxueqing>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, eric, esprehn, kennyluck, ojan.autocc, tabatkins, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 62048    
Attachments:
Description Flags
patch
webkit.review.bot: commit-queue-
patch
webkit.review.bot: commit-queue-
patch
webkit.review.bot: commit-queue-
patch
none
patch none

Description huangxueqing 2012-12-09 08:03:15 PST
Flexbox should ignore firstLine pseudo element.
testcase:
<style> 
	div { display: -webkit-flex;  display: -moz-flex; display: flex; } 
	div::first-line { font-size: 10em;} 
	p:first-child { -webkit-order: 1; -moz-order: 1; order: 1;}
</style>
<div>
	<p>The first item.</p>
	<p>The second item.</p>
</div>
In above case, the flex item <p>(both the firet one and the second) should ignore "font-size: 10em;"

Spec[1] said that "None of the properties defined in this module apply to the ‘::first-line’ or ‘::first-letter’ pseudo-elements."
In addition, css2[2] define that "The :first-line pseudo-element can only be attached to a block container element."

opera 12.11, IE 10 and firefox 20 alpha have correct behavior.

[1]http://dev.w3.org/csswg/css3-flexbox/#display-flex
[2]http://www.w3.org/TR/CSS2/selector.html#first-line-pseudo
Comment 1 huangxueqing 2012-12-09 16:52:02 PST
Created attachment 178452 [details]
patch
Comment 2 Kang-Hao (Kenny) Lu 2012-12-09 22:14:00 PST
(In reply to comment #0)
> Spec[1] said that "None of the properties defined in this module apply to the
> ‘::first-line’ or ‘::first-letter’ pseudo-elements."

Yes, but 'font-size' isn't a property defined in this module. I think the relevant prose here is this:

  # Flex containers are not block containers,...

> In addition, css2[2] define that "The :first-line pseudo-element can only be
> attached to a block container element."
> 
> [1]http://dev.w3.org/csswg/css3-flexbox/#display-flex
Comment 3 huangxueqing 2012-12-09 22:40:46 PST
(In reply to comment #2)
> (In reply to comment #0)
> > Spec[1] said that "None of the properties defined in this module apply to the
> > ‘::first-line’ or ‘::first-letter’ pseudo-elements."
> 
> Yes, but 'font-size' isn't a property defined in this module. I think the relevant prose here is this:
> 
>   # Flex containers are not block containers,...
> 
Exactly.

> > In addition, css2[2] define that "The :first-line pseudo-element can only be
> > attached to a block container element."
> > 
> > [1]http://dev.w3.org/csswg/css3-flexbox/#display-flex
Comment 4 WebKit Review Bot 2012-12-10 02:40:16 PST
Comment on attachment 178452 [details]
patch

Attachment 178452 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15223604

New failing tests:
css3/flexbox/inline-flex-ignore-firstLine.html
css3/flexbox/flex-ignore-firstLine.html
css3/flexbox/inline-flex-item-firstLine-valid.html
css3/flexbox/flex-item-firstLine-valid.html
Comment 5 Build Bot 2012-12-10 08:27:58 PST
Comment on attachment 178452 [details]
patch

Attachment 178452 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15230647

New failing tests:
css3/flexbox/inline-flex-item-firstLine-valid.html
Comment 6 huangxueqing 2012-12-11 07:36:04 PST
Created attachment 178802 [details]
patch

It seems that font-size has different height in Chromium, i change test case use line-height to insteat it.
Comment 7 WebKit Review Bot 2012-12-11 08:41:14 PST
Comment on attachment 178802 [details]
patch

Attachment 178802 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15278156

New failing tests:
css3/flexbox/inline-flex-item-firstLine-valid.html
Comment 8 WebKit Review Bot 2012-12-11 09:45:48 PST
Comment on attachment 178802 [details]
patch

Attachment 178802 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15278181

New failing tests:
css3/flexbox/inline-flex-item-firstLine-valid.html
Comment 9 huangxueqing 2012-12-11 18:54:13 PST
Created attachment 178944 [details]
patch
Comment 10 WebKit Review Bot 2012-12-11 20:41:35 PST
Comment on attachment 178944 [details]
patch

Attachment 178944 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15280334

New failing tests:
css3/flexbox/inline-flex-item-firstLine-valid.html
Comment 11 huangxueqing 2012-12-11 22:10:53 PST
Created attachment 178965 [details]
patch
Comment 12 huangxueqing 2012-12-15 19:59:49 PST
Created attachment 179639 [details]
patch
Comment 13 Tony Chang 2012-12-18 14:04:18 PST
(In reply to comment #0)
> Flexbox should ignore firstLine pseudo element.
> testcase:
> <style> 
>     div { display: -webkit-flex;  display: -moz-flex; display: flex; } 
>     div::first-line { font-size: 10em;} 
>     p:first-child { -webkit-order: 1; -moz-order: 1; order: 1;}
> </style>
> <div>
>     <p>The first item.</p>
>     <p>The second item.</p>
> </div>
> In above case, the flex item <p>(both the firet one and the second) should ignore "font-size: 10em;"
> 
> Spec[1] said that "None of the properties defined in this module apply to the ‘::first-line’ or ‘::first-letter’ pseudo-elements."
> In addition, css2[2] define that "The :first-line pseudo-element can only be attached to a block container element."

Are you sure that's what this is saying?  I think the spec means that you can't, e.g., do
div::first-line {
  -webkit-order: 2;
}

in an attempt to move div::first-line.  I don't see why the font-size shouldn't apply here.
Comment 14 Tab Atkins 2012-12-18 14:09:22 PST
(In reply to comment #13)
> (In reply to comment #0)
> > Flexbox should ignore firstLine pseudo element.
> > testcase:
> > <style> 
> >     div { display: -webkit-flex;  display: -moz-flex; display: flex; } 
> >     div::first-line { font-size: 10em;} 
> >     p:first-child { -webkit-order: 1; -moz-order: 1; order: 1;}
> > </style>
> > <div>
> >     <p>The first item.</p>
> >     <p>The second item.</p>
> > </div>
> > In above case, the flex item <p>(both the firet one and the second) should ignore "font-size: 10em;"
> > 
> > Spec[1] said that "None of the properties defined in this module apply to the ‘::first-line’ or ‘::first-letter’ pseudo-elements."
> > In addition, css2[2] define that "The :first-line pseudo-element can only be attached to a block container element."
> 
> Are you sure that's what this is saying?  I think the spec means that you can't, e.g., do
> div::first-line {
>   -webkit-order: 2;
> }
> 
> in an attempt to move div::first-line.  I don't see why the font-size shouldn't apply here.

Tony is correct.  Read the line again - the properties *in this spec* don't apply to ::first-line or ::first-letter.  That just mean you can't do something like Tony's example.  

CSS 2.1's text was written before Flexbox had been created, so don't read too much into the precise term "block container element".  It's actually undefined whether ::first-line/letter should apply to a flexbox.  I'll ask on www-style.
Comment 15 huangxueqing 2012-12-18 18:21:31 PST
(In reply to comment #14)
> (In reply to comment #13)
> > (In reply to comment #0)
> > > Flexbox should ignore firstLine pseudo element.
> > > testcase:
> > > <style> 
> > >     div { display: -webkit-flex;  display: -moz-flex; display: flex; } 
> > >     div::first-line { font-size: 10em;} 
> > >     p:first-child { -webkit-order: 1; -moz-order: 1; order: 1;}
> > > </style>
> > > <div>
> > >     <p>The first item.</p>
> > >     <p>The second item.</p>
> > > </div>
> > > In above case, the flex item <p>(both the firet one and the second) should ignore "font-size: 10em;"
> > > 
> > > Spec[1] said that "None of the properties defined in this module apply to the ‘::first-line’ or ‘::first-letter’ pseudo-elements."
> > > In addition, css2[2] define that "The :first-line pseudo-element can only be attached to a block container element."
> > 
> > Are you sure that's what this is saying?  I think the spec means that you can't, e.g., do
> > div::first-line {
> >   -webkit-order: 2;
> > }
> > 
> > in an attempt to move div::first-line.  I don't see why the font-size shouldn't apply here.
> 
> Tony is correct.  Read the line again - the properties *in this spec* don't apply to ::first-line or ::first-letter.  That just mean you can't do something like Tony's example.  
> 
> CSS 2.1's text was written before Flexbox had been created, so don't read too much into the precise term "block container element".  It's actually undefined whether ::first-line/letter should apply to a flexbox.  I'll ask on www-style.

Your are totally correct, it's undefined in specs.
What i mean was that it's make no sense that flex item apply ::first-line or ::first-letter, as above example, "-webkit-order: 1" make "The first item." was not the first child of div in term of views (behind of "The second item."), it apply "font-size: 10em" was confusion. Maybe this was wrong place to discuss this problem.
Futher, for undefined things, should we make webkit consistent with mozilla, opera and IE 10?
Comment 16 Tony Chang 2013-02-15 12:46:02 PST
Comment on attachment 179639 [details]
patch

The CSS WG decided that first-line and first-letter should not apply to flex containers:
http://dev.w3.org/csswg/css3-flexbox/issues-cr-2012#issue-13
Comment 17 WebKit Review Bot 2013-02-15 13:18:20 PST
Comment on attachment 179639 [details]
patch

Clearing flags on attachment: 179639

Committed r143042: <http://trac.webkit.org/changeset/143042>
Comment 18 WebKit Review Bot 2013-02-15 13:18:25 PST
All reviewed patches have been landed.  Closing bug.