Currently both RenderTextFragment and RenderText for first letter point to the same Text node. There should only be one non-anonymous renderer per node.
Created attachment 325569 [details] patch
Attachment 325569 [details] did not pass style-queue: ERROR: Source/WebCore/rendering/RenderBoxModelObject.cpp:2553: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 1 in 57 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 325570 [details] patch
Created attachment 325571 [details] patch
Comment on attachment 325571 [details] patch Attachment 325571 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5062417 Number of test failures exceeded the failure limit.
Created attachment 325573 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 325571 [details] patch Attachment 325571 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/5062479 New failing tests: fast/css-generated-content/initial-letter-border-padding.html fast/css-generated-content/initial-letter-basic.html fast/css-generated-content/initial-letter-raised.html fast/css-generated-content/initial-letter-sunken.html ietestcenter/css3/text/textshadow-004.htm
Created attachment 325574 [details] Archive of layout-test-results from ews100 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 325571 [details] patch Attachment 325571 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/5062493 New failing tests: ietestcenter/css3/text/textshadow-004.htm fast/css-generated-content/initial-letter-basic.html imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-match.https.html svg/wicd/test-rightsizing-a.xhtml fast/css-generated-content/initial-letter-raised.html fast/css-generated-content/initial-letter-border-padding.html fast/css-generated-content/initial-letter-sunken.html
Created attachment 325575 [details] Archive of layout-test-results from ews106 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Created attachment 325576 [details] patch
Comment on attachment 325576 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=325576&action=review > Source/WebCore/rendering/RenderBoxModelObject.cpp:131 > +typedef HashMap<const RenderBoxModelObject*, WeakPtr<RenderTextFragment>> FirstLetterRemainingTextMap; In new code it’s nice to use "using" instead of typedef.
Comment on attachment 325576 [details] patch We should tighten the first letter type here. It's odd that we set it on the RenderElement but use it in RenderBoxModelObject for cleanup (I know it's not a new code). I also believe we could even use a more concrete renderer type for all this.
Comment on attachment 325576 [details] patch Attachment 325576 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5062977 New failing tests: ietestcenter/css3/text/textshadow-004.htm fast/selectors/166a.html fast/text/firstline/003.html fast/css/first-letter-first-line-hover.html fast/css/first-letter-hover.html fast/css/first-letter-detach.html fast/css/first-letter-float-after-float.html fast/css/first-letter-visibility.html fast/css/first-letter-capitalized.html fast/css/first-letter-recalculation.html fast/css/first-letter-float.html fast/selectors/039b.html fast/multicol/shadow-breaking.html fast/text/firstline/002.html fast/css/first-letter-punctuation.html fast/selectors/039.html
Created attachment 325581 [details] Archive of layout-test-results from ews123 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
(In reply to zalan from comment #13) > Comment on attachment 325576 [details] > patch > > We should tighten the first letter type here. It's odd that we set it on the > RenderElement but use it in RenderBoxModelObject for cleanup (I know it's > not a new code). I also believe we could even use a more concrete renderer > type for all this. RenderBoxModelObject is the most concrete type of the first letter renderer as it can be either RenderInline or RenderBlock. RenderElement is used for bits simply because that's where we currently keep all the bits.
> RenderElement is used for bits simply because that's where we currently keep > all the bits. We could make the setters available only in RBMO though.
Comment on attachment 325576 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=325576&action=review > Source/WebCore/rendering/RenderElement.h:344 > + unsigned m_isFirstLetter : 1; Is it worth using a RenderElement bit here? I think first letter is rare enough that you could use RenderObject::rareData().
> Is it worth using a RenderElement bit here? I think first letter is rare > enough that you could use RenderObject::rareData(). I don't think so. There are plenty of free bits in RenderElement and we could probably just add more too (renderer size doesn't appear to be a huge concern in memory or performance). I'm also unconvinced that RareData is a good pattern in general. As you pile up stuff there you'll end up with a data structure that is large and not rare. See ElementRareData. The traditional feature-bit-and-side-hashmap approach used all over render tree seems strictly better to me.
Created attachment 325626 [details] patch
Comment on attachment 325626 [details] patch Attachment 325626 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5066822 New failing tests: fast/selectors/039b.html fast/selectors/039.html
Created attachment 325639 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Created attachment 325696 [details] patch
Comment on attachment 325696 [details] patch Clearing flags on attachment: 325696 Committed r224324: <https://trac.webkit.org/changeset/224324>
All reviewed patches have been landed. Closing bug.
<rdar://problem/35567715>