[Webkit-unassigned] [Bug 80794] :first-line pseudo selector ignoring words created from :before

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=80794


Daniel Bates <dbates at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #167255|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #26 from Daniel Bates <dbates at webkit.org>  2012-10-06 15:28:10 PST ---
(From update of attachment 167255)
View in context: https://bugs.webkit.org/attachment.cgi?id=167255&action=review

> Source/WebCore/ChangeLog:30
> +        helper function getFirstLineStyle().

getFirstLineStyle() => firstLineStyleForCachedUncachedType()

> LayoutTests/fast/css/first-line-style-for-before-after-content-expected.html:13
> +p:first-line {
> +    color: green;
> +}
> +p:first-letter {
> +    color: blue;
> +}	

It bothers me that we are using :first-line and :first-letter in this expected results file given that this patch fixes an issue with generated content/pseudo-elements. Although this expected results file uses existing machinery, including machinery we preserved in this patch, it is preferred that we construct the expected results without the use of the :first-letter and :first-line pseudo-elements so as to further differentiate the code path taken to generate the actual and expected results of the test, respectively. Can we come up with a way to generate these expected results without using :first-line and :first-letter? Maybe construct the contents of the HTML p elements (below) using stylized <span>s?

> LayoutTests/fast/css/first-line-style-for-before-after-content-expected.html:14
> +</style>

Please remove the tab character at the end of the previous line.

> LayoutTests/fast/css/first-line-style-for-before-after-content-expected.html:19
> +    <br/>

Nit: No need to explicitly terminate the br element in an HTML5 document. We can write this as <br>.

> LayoutTests/fast/css/first-line-style-for-before-after-content.html:25
> +}	

Please remove the tab character at the end of this line.

> LayoutTests/fast/css/first-line-style-for-before-after-content.html:31
> +    <br/>

Nit: No need to explicitly terminate the br element in an HTML5 document. We can write this as <br>.

> LayoutTests/fast/css/first-line-style-for-before-after-content.html:37
> +    <p><span id="before"> of this paragraph should be displayed in green color.</span></p>

This is invalid markup by uniqueness of the HTML attribute id, <http://www.whatwg.org/specs/web-apps/current-work/multipage/elements.html#the-id-attribute>. That is, the HTML attribute id for the <span> must be unique, but it isn't. The first occurrence of this id was on line 34.

> LayoutTests/fast/css/first-line-style-for-before-after-content.html:43
> +    <p><span id="after">Display </span></p>

This is invalid markup by uniqueness of the HTML attribute id, <http://www.whatwg.org/specs/web-apps/current-work/multipage/elements.html#the-id-attribute>. That is, the HTML attribute id for the <span> must be unique, but it isn't. The first occurrence of this id was on line 40.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.