[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





--- Comment #22 from Arpita Bahuguna <arpitabahuguna at gmail.com>  2012-10-04 23:19:32 PST ---
(In reply to comment #19)
> (From update of attachment 166838 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=166838&action=review
> 
Thanks for the review Daniel.

> > Source/WebCore/rendering/RenderObject.cpp:2607
> > +static PassRefPtr<RenderStyle> getFirstLineStyle(StyleCacheState type, const RenderObject* renderer, RenderStyle* style)
> 
> This is a very minor nit. We tend to prefix functions with the word "get" when it returns one or more values through out arguments (*). Notice that this function doesn't return any value through an out argument. Although we have functions that don't follow this convention, including RenderObject::{getCachedPseudoStyle, getUncachedPseudoStyle}(), it would be great if we could come up with a descriptive name for this function that omitted the prefix "get".
> 
> (*) See <http://www.webkit.org/coding/coding-style.html#names-out-argument>.
> 
I tried to come up with other alternative names such as, firstLineStyleForCacheType, deriveFirstLineStyle etc. but firstLineStyle() seemed to be the most appropriate. Since that is already in use, have currently changed the name to firstLineStyleForCachedUncachedType since I understand prefixing the function name with 'get' here would not be correct.

> > Source/WebCore/rendering/RenderObject.cpp:2618
> > +            // A first-line style is in effect. Cache a first-line style for ourselves.
> 
> I suggest that we move this comment inside the block associated with the "if (type == Cached)" statement (below) so as to scope it to that code.
> 
Done.

> > Source/WebCore/rendering/RenderObject.cpp:2629
> > +            return firstLineBlock->getUncachedPseudoStyle(FIRST_LINE, style, firstLineBlock == renderer ? style : 0);
> 
> Can you give an example when firstLineBlock == renderer evaluates to true?

After verification of the condition I realized that we need to be checking for firstLineBlock == renderer->parent() for this particular case, if required at all. This lead to further checking with various other scenarios and unfortunately my fix was still not handling the following case correctly:

<p id="firstline"><span id="before">Content</span> </p>
Here our <p> tag has :first-line specified for it and our <span> has :before specified for it.

For such a case, my previous fix was insufficient as it would only check for the parent renderer to be a blockflow. But in case (as shown above) our parent renderer (for the generated :before/:after content) is a RenderInline, we would still not apply the first-line style to this content.

Thus, it basically implies that if in case our renderer has before or after content, we should use its parent and carry out the same checks as were done previously, i.e. for both block flow as well as inline.

Have made the changes accordingly in the latest patch; hope this shall be more appropriate.

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