Bug 101171 – [Shadow][Refactoring] Remove shadowPseudoId() and use pseudo() instead

• Status: RESOLVED
• Resolution: WONTFIX
• Component: DOM

[Shinya Kawanaka]

We would like to remove shadowPseudoId() and use pseudo() instead.

[Shinya Kawanaka]

Created attachment 172460 [details]
WIP

[Build Bot]

Comment on attachment 172460 [details]
WIP

Attachment 172460 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14726818

[Build Bot]

Comment on attachment 172460 [details]
WIP

Attachment 172460 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14691306

[Shinya Kawanaka]

Created attachment 172475 [details]
WIP

[Kent Tamura]

Comment on attachment 172475 [details]
WIP

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

> Source/WebCore/html/ColorInputType.cpp:121
> -    wrapperElement->setShadowPseudoId("-webkit-color-swatch-wrapper");
> +    wrapperElement->setPseudo("-webkit-color-swatch-wrapper");

nit: If you have enough time, I recommend to use AtomicString("-webkit-color-swatch-wrapper", AtomicString::ConstructFromLiteral)
http://trac.webkit.org/wiki/EfficientStrings

> Source/WebCore/html/FileInputType.cpp:86
> -const AtomicString& UploadButtonElement::shadowPseudoId() const
> +const AtomicString& UploadButtonElement::pseudo() const
>  {
>      DEFINE_STATIC_LOCAL(AtomicString, pseudoId, ("-webkit-file-upload-button"));
>      return pseudoId;

Overriding "pseudo" looks strange.  Why don't you convert such code to setPseudo?

[Shinya Kawanaka]

(In reply to comment #5)
> (From update of attachment 172475 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=172475&action=review
> 
> > Source/WebCore/html/ColorInputType.cpp:121
> > -    wrapperElement->setShadowPseudoId("-webkit-color-swatch-wrapper");
> > +    wrapperElement->setPseudo("-webkit-color-swatch-wrapper");
> 
> nit: If you have enough time, I recommend to use AtomicString("-webkit-color-swatch-wrapper", AtomicString::ConstructFromLiteral)
> http://trac.webkit.org/wiki/EfficientStrings
> 
> > Source/WebCore/html/FileInputType.cpp:86
> > -const AtomicString& UploadButtonElement::shadowPseudoId() const
> > +const AtomicString& UploadButtonElement::pseudo() const
> >  {
> >      DEFINE_STATIC_LOCAL(AtomicString, pseudoId, ("-webkit-file-upload-button"));
> >      return pseudoId;
> 
> Overriding "pseudo" looks strange.  Why don't you convert such code to setPseudo?

I'll convert them... since pseudo() is not virtual at all.

[Build Bot]

Comment on attachment 172475 [details]
WIP

Attachment 172475 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14725877

[Shinya Kawanaka]

Since we have a lot of places to be replaced, I would like to split this to several patches...

[kov's GTK+ EWS bot]

Comment on attachment 172475 [details]
WIP

Attachment 172475 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/14731694

[Build Bot]

Comment on attachment 172475 [details]
WIP

Attachment 172475 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14665460

[Ryosuke Niwa]

We're not gonna do this. V1 API allows ::part, which is distant from pseudo element mechanism.