RESOLVED CONFIGURATION CHANGED 30708
::first-letter inherits from ::after pseudo-element
https://bugs.webkit.org/show_bug.cgi?id=30708
Summary ::first-letter inherits from ::after pseudo-element
Philippe Wittenbergh
Reported 2009-10-23 00:17:47 PDT
Created attachment 41717 [details] test case ::first-letter inherits properties from ::after pseudo-element The following properties are inherited (unless overridden by the ::first-letter selector): * color (but not background-color) * font-size * font-weight * font-style * text-transform Non-exhaustive list, that is what I've tested so far :-) This is similar to the inheritance problems noted in bug 21937 - with display:run-in. In the testcase: AR: the first-letter is red, italicised and smaller than the subsequent text, and lower-cased… ER: the first 'L' should be identical to the second one, except bold. Gecko, Opera display this correctly.
Attachments
test case (782 bytes, text/html)
2009-10-23 00:17 PDT, Philippe Wittenbergh
no flags
Proposed Patch (3.04 KB, patch)
2010-05-28 02:55 PDT, Yoshiki Hayashi
zimmermann: review-
Philippe Wittenbergh
Comment 1 2009-10-23 01:41:09 PDT
btw, this displays correctly in Safari 2.04, but fails in 3.04 and anything newer.
Yoshiki Hayashi
Comment 2 2010-05-28 02:53:14 PDT
I investigated the bug and found that updateFirstLetter is called twice on the same node, one coming from HTMLParser::insertNode and other from RenderView::layout. I copy pasted two stack traces below. With some printf debugging, what I found out was that in the first call, the P node which has both :first-letter and :after has only the content of :after as a child and not the body of P. So when updateFirstLetter is called, the first letter block inherits from the first line style which happens to be the style of :after and thus it inherits those properties instead of the one from the body of P. The fix was simple, just removing updateFirstLetter call from styleDidChange as it will be called from the first thing in the layout anyways. I'm going to attach a patch for this. The included test depends on bug https://bugs.webkit.org/show_bug.cgi?id=39863 because otherwise first-letter doesn't show up in dumpAsText. # 1st call #0 WebCore::RenderBlock::updateFirstLetter (this=0xbed878) at ../../../WebCore/rendering/RenderBlock.cpp:4639 #1 0x00007ffff672e906 in WebCore::RenderBlock::styleDidChange (this=0xbed878, diff=WebCore::StyleDifferenceEqual, oldStyle=0x0) at ../../../WebCore/rendering/RenderBlock.cpp:251 #2 0x00007ffff67a988f in WebCore::RenderObject::setStyle (this=0xbed878, style=...) at ../../../WebCore/rendering/RenderObject.cpp:1545 #3 0x00007ffff67a73a0 in WebCore::RenderObject::setAnimatableStyle ( this=0xbed878, style=...) at ../../../WebCore/rendering/RenderObject.cpp:1479 #4 0x00007ffff6347d5b in WebCore::Node::createRendererIfNeeded (this=0xbb7b40) at ../../../WebCore/dom/Node.cpp:1359 #5 0x00007ffff63175df in WebCore::Element::attach (this=0xbb7b40) at ../../../WebCore/dom/Element.cpp:820 #6 0x00007ffff64a8174 in WebCore::HTMLParser::insertNode (this=0xbe5650, n=0xbb7b40, flat=false) at ../../../WebCore/html/HTMLParser.cpp:403 #7 0x00007ffff64aa726 in WebCore::HTMLParser::insertNodeAfterLimitDepth ( this=0xbe5650, n=0xbb7b40, flat=false) at ../../../WebCore/html/HTMLParser.cpp:234 #8 0x00007ffff64aaf53 in WebCore::HTMLParser::parseToken (this=0xbe5650, t=0xb5e5f8) at ../../../WebCore/html/HTMLParser.cpp:306 #9 0x00007ffff64bfe59 in WebCore::HTMLTokenizer::processToken (this=0xbc0ee0) #10 0x00007ffff64c814f in WebCore::HTMLTokenizer::parseTag (this=0xbc0ee0, src=..., state=...) at ../../../WebCore/html/HTMLTokenizer.cpp:1513 #11 0x00007ffff64c8cae in WebCore::HTMLTokenizer::advance (this=0xbc0ee0, state=...) at ../../../WebCore/html/HTMLTokenizer.cpp:1702 #2nd call #0 WebCore::RenderBlock::updateFirstLetter (this=0xbed878) at ../../../WebCore/rendering/RenderBlock.cpp:4639 #1 0x00007ffff672d052 in WebCore::RenderBlock::layout (this=0xbed878) at ../../../WebCore/rendering/RenderBlock.cpp:667 #2 0x00007ffff672bf4d in WebCore::RenderBlock::layoutBlockChild ( this=0xbf00b8, child=0xbed878, marginInfo=..., previousFloatBottom=@0x7fffffffbed4, maxFloatBottom=@0x7fffffffc024) at ../../../WebCore/rendering/RenderBlock.cpp:1364 #3 0x00007ffff672cd78 in WebCore::RenderBlock::layoutBlockChildren ( this=0xbf00b8, relayoutChildren=true, maxFloatBottom=@0x7fffffffc024) at ../../../WebCore/rendering/RenderBlock.cpp:1307 #4 0x00007ffff672d777 in WebCore::RenderBlock::layoutBlock (this=0xbf00b8, relayoutChildren=true) at ../../../WebCore/rendering/RenderBlock.cpp:749 #5 0x00007ffff672d06d in WebCore::RenderBlock::layout (this=0xbf00b8) at ../../../WebCore/rendering/RenderBlock.cpp:671 #6 0x00007ffff672bf4d in WebCore::RenderBlock::layoutBlockChild ( this=0xbee9e8, child=0xbf00b8, marginInfo=..., previousFloatBottom=@0x7fffffffc294, maxFloatBottom=@0x7fffffffc3e4) at ../../../WebCore/rendering/RenderBlock.cpp:1364 #7 0x00007ffff672cd78 in WebCore::RenderBlock::layoutBlockChildren ( this=0xbee9e8, relayoutChildren=true, maxFloatBottom=@0x7fffffffc3e4) at ../../../WebCore/rendering/RenderBlock.cpp:1307 #8 0x00007ffff672d777 in WebCore::RenderBlock::layoutBlock (this=0xbee9e8, relayoutChildren=true) at ../../../WebCore/rendering/RenderBlock.cpp:749 #9 0x00007ffff672d06d in WebCore::RenderBlock::layout (this=0xbee9e8) at ../../../WebCore/rendering/RenderBlock.cpp:671 #10 0x00007ffff672bf4d in WebCore::RenderBlock::layoutBlockChild ( this=0xbe53b8, child=0xbee9e8, marginInfo=..., previousFloatBottom=@0x7fffffffc654, maxFloatBottom=@0x7fffffffc7a4) at ../../../WebCore/rendering/RenderBlock.cpp:1364 #11 0x00007ffff672cd78 in WebCore::RenderBlock::layoutBlockChildren ( this=0xbe53b8, relayoutChildren=true, maxFloatBottom=@0x7fffffffc7a4) at ../../../WebCore/rendering/RenderBlock.cpp:1307 #12 0x00007ffff672d777 in WebCore::RenderBlock::layoutBlock (this=0xbe53b8, relayoutChildren=true) at ../../../WebCore/rendering/RenderBlock.cpp:749 #13 0x00007ffff672d06d in WebCore::RenderBlock::layout (this=0xbe53b8) at ../../../WebCore/rendering/RenderBlock.cpp:671 #14 0x00007ffff67f0dbc in WebCore::RenderView::layout (this=0xbe53b8) at ../../../WebCore/rendering/RenderView.cpp:123 #15 0x00007ffff6619aec in WebCore::FrameView::layout (this=0xb5f470, allowSubtree=true) at ../../../WebCore/page/FrameView.cpp:749 #16 0x00007ffff62fc0a5 in WebCore::Document::implicitClose (this=0xbc0ee0) at ../../../WebCore/dom/Document.cpp:1884 #17 0x00007ffff656e051 in WebCore::FrameLoader::checkCallImplicitClose (
Yoshiki Hayashi
Comment 3 2010-05-28 02:55:56 PDT
Created attachment 57309 [details] Proposed Patch
Nikolas Zimmermann
Comment 4 2010-07-30 23:18:09 PDT
Comment on attachment 57309 [details] Proposed Patch LayoutTests/fast/css/first-letter-and-after-expected.txt:1 + Test to make sure :first-letter do not inherit from :after. If you see "testAFTER", the test has passed. I'm not seeing "testAFTER", but "test". Does the testcase actually work? WebCore/rendering/RenderBlock.cpp:256 + // Intentionally not calling updateFirstLetter because it will be done first thing in the morning in layout(). I hope it's also called in the afternoon... just leave it out. "Intentionally not calling updateFirstLetter() because it's done in layout().
Elliott Sprehn
Comment 5 2012-10-04 10:49:31 PDT
This has since been fixed, but I'm not sure we ever got a test for it. Someone should upload this patch again with just the test :)
Eric Seidel (no email)
Comment 6 2012-10-04 10:55:27 PDT
rs=me on landing just the test.
Ahmad Saleem
Comment 7 2022-07-24 08:16:39 PDT
It is mentioned that this issue has been fixed in Comment 05 and I can confirm that all browsers (Chrome Canary 106, Firefox Nightly 104 and Safari 15.6) match with each other. But I am not sure whether the test cases landed or not, although WPT has quite a lot of coverage of these: https://wpt.fyi/results/css/css-pseudo?label=master&label=experimental&aligned&q=first-letter So I am not sure whether there is a specific need to add test when WPT tests are already imported. Just wanted to share updated perspective. Please mark it as "RESOLVED WONTFIX" or "RESOLVED CONFIGURATION CHANGED" or else as needed. Thanks!
Ryosuke Niwa
Comment 8 2022-07-24 14:02:35 PDT
Config changed. I don't think we need to bother with tests at this point.
Note You need to log in before you can comment on or make changes to this bug.