Bug 120204

Summary: Tighten before/after pseudo element accessors
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, commit-queue, esprehn+autocc, glenn, gtk-ews, kangil.han, kling, kondapallykalyan, macpherson, menard, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
gtk-ews: commit-queue-
another kling: review+, gtk-ews: commit-queue-

Description Antti Koivisto 2013-08-23 04:19:57 PDT 
We have generic looking Element::pseudoElement(PseudoID) which only returns before/after pseudo elements.
Comment 1 Antti Koivisto 2013-08-23 04:33:13 PDT 
Created attachment 209448 [details]
patch
Comment 2 kov's GTK+ EWS bot 2013-08-23 05:34:33 PDT 
Comment on attachment 209448 [details]
patch

Attachment 209448 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/1528576
Comment 3 Antti Koivisto 2013-08-24 00:14:10 PDT 
Created attachment 209534 [details]
another
Comment 4 kov's GTK+ EWS bot 2013-08-24 01:27:48 PDT 
Comment on attachment 209534 [details]
another

Attachment 209534 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/1529786
Comment 5 kov's GTK+ EWS bot 2013-08-24 02:46:59 PDT 
Comment on attachment 209534 [details]
another

Attachment 209534 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/1557420
Comment 6 Andreas Kling 2013-08-24 03:51:28 PDT 
Comment on attachment 209534 [details]
another

View in context: https://bugs.webkit.org/attachment.cgi?id=209534&action=review

r=me

> Source/WebCore/dom/Element.cpp:2069
> +    if (pseudoElementSpecifier != BEFORE && pseudoElementSpecifier != AFTER)
> +        return 0;
> +    return pseudoElementSpecifier == BEFORE ? host->beforePseudoElement() : host->afterPseudoElement();

This might look better as a switch.

> Source/WebCore/dom/Element.cpp:2310
> +bool Element::updateExistingPseudoElement(PseudoElement* existing, Style::Change change)

existing -> existingPseudoElement?

> Source/WebCore/dom/Element.cpp:2341
> +    if (PseudoElement* existing = beforePseudoElement()) {

existing -> existingPseudoElement?

> Source/WebCore/dom/Element.cpp:2352
> +    if (PseudoElement* existing = afterPseudoElement()) {

existing -> existingPseudoElement?

> Source/WebCore/dom/Element.cpp:2396
> +    if (!hasRareData())
> +        return;

This could be an assertion instead, if we moved the two clearFooPseudoElement() calls in clearStyleDerived... after the hasRareData check.

> Source/WebCore/dom/Element.cpp:2404
> +    if (!hasRareData())
> +        return;

Ditto.
Comment 7 Antti Koivisto 2013-08-24 04:15:11 PDT 
https://trac.webkit.org/r154541