Bug 102349 – :enabled should match with an anchor element having href attribute

• Component: CSS
• Status: RESOLVED INVALID

[Shinya Kawanaka]

According to HTML5 spec,
http://www.whatwg.org/specs/web-apps/current-work/multipage/selectors.html#selector-enabled

:enabled should match with anchor/area/link having href attribute.
However, it does not match now.

[Shinya Kawanaka]

link element is moved to https://bugs.webkit.org/show_bug.cgi?id=102351

[Shinya Kawanaka]

Created attachment 174368 [details]
Patch

[Allan Sandfeld Jensen]

Comment on attachment 174368 [details]
Patch

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

> Source/WebCore/css/SelectorChecker.cpp:1042
> -            if (element && (element->isFormControlElement() || element->hasTagName(optionTag) || element->hasTagName(optgroupTag)))
> +            if (element && (element->isFormControlElement() || element->hasTagName(optionTag) || element->hasTagName(optgroupTag) || element->isLink()))

Won't this match the parent image of the AREA rather than the AREA element? That is any IMG element with a USEMAP property.

[Shinya Kawanaka]

Comment on attachment 174368 [details]
Patch

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

>> Source/WebCore/css/SelectorChecker.cpp:1042
>> +            if (element && (element->isFormControlElement() || element->hasTagName(optionTag) || element->hasTagName(optgroupTag) || element->isLink()))
> 
> Won't this match the parent image of the AREA rather than the AREA element? That is any IMG element with a USEMAP property.

Ah, thanks! We have to check tagName.

[Shinya Kawanaka]

Comment on attachment 174368 [details]
Patch

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

>>> Source/WebCore/css/SelectorChecker.cpp:1042
>>> +            if (element && (element->isFormControlElement() || element->hasTagName(optionTag) || element->hasTagName(optgroupTag) || element->isLink()))
>> 
>> Won't this match the parent image of the AREA rather than the AREA element? That is any IMG element with a USEMAP property.
> 
> Ah, thanks! We have to check tagName.

By the way this implementation is completely wrong :-(

[Allan Sandfeld Jensen]

(In reply to comment #5)
> (From update of attachment 174368 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=174368&action=review
> 
> >>> Source/WebCore/css/SelectorChecker.cpp:1042
> >>> +            if (element && (element->isFormControlElement() || element->hasTagName(optionTag) || element->hasTagName(optgroupTag) || element->isLink()))
> >> 
> >> Won't this match the parent image of the AREA rather than the AREA element? That is any IMG element with a USEMAP property.
> > 
> > Ah, thanks! We have to check tagName.
> 
> By the way this implementation is completely wrong :-(

The more I think about it, I guess you should skip handling AREA elements to begin with. It seems they haven't thought it through in the standard. AREAs at least in WebKit do not even have their own render object, and will therefore not have their own style either, even if we manage to set a specific style for them in ElementRareData, it will not be used, because AREAs are not rendered.

[Shinya Kawanaka]

Created attachment 174384 [details]
Patch

[Shinya Kawanaka]

(In reply to comment #6)
> (In reply to comment #5)
> > (From update of attachment 174368 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=174368&action=review
> > 
> > >>> Source/WebCore/css/SelectorChecker.cpp:1042
> > >>> +            if (element && (element->isFormControlElement() || element->hasTagName(optionTag) || element->hasTagName(optgroupTag) || element->isLink()))
> > >> 
> > >> Won't this match the parent image of the AREA rather than the AREA element? That is any IMG element with a USEMAP property.
> > > 
> > > Ah, thanks! We have to check tagName.
> > 
> > By the way this implementation is completely wrong :-(
> 
> The more I think about it, I guess you should skip handling AREA elements to begin with. It seems they haven't thought it through in the standard. AREAs at least in WebKit do not even have their own render object, and will therefore not have their own style either, even if we manage to set a specific style for them in ElementRareData, it will not be used, because AREAs are not rendered.

Hmm... the reason why AREA is not rendered is just AREA has display: none.
Actually my test renders AREA.

[Shinya Kawanaka]

This is an output of DRT

Content-Type: text/plain
layer at (0,0) size 800x600
  RenderView at (0,0) size 800x600
layer at (0,0) size 800x196
  RenderBlock {HTML} at (0,0) size 800x196
    RenderBody {BODY} at (8,16) size 784x164
      RenderBlock {P} at (0,0) size 784x20
        RenderText {#text} at (0,0) size 448x19
          text run at (0,0) width 448: ":enabled should match with an anchor or area element having href attribute."
      RenderBlock {P} at (0,36) size 784x20
        RenderInline {A} at (0,0) size 196x19 [color=#0000EE] [bgcolor=#0000FF]
          RenderText {#text} at (0,0) size 196x19
            text run at (0,0) width 196: "This background should be blue."
      RenderBlock {P} at (0,72) size 784x20
        RenderInline {A} at (0,0) size 219x19
          RenderText {#text} at (0,0) size 219x19
            text run at (0,0) width 219: "This background should not be blue."
      RenderBlock {P} at (0,108) size 784x20
        RenderInline {AREA} at (0,0) size 196x19 [bgcolor=#0000FF]
          RenderText {#text} at (0,0) size 196x19
            text run at (0,0) width 196: "This background should be blue."
      RenderBlock {P} at (0,144) size 784x20
        RenderInline {AREA} at (0,0) size 219x19
          RenderText {#text} at (0,0) size 219x19
            text run at (0,0) width 219: "This background should not be blue."
#EOF

[Allan Sandfeld Jensen]

(In reply to comment #8)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > (From update of attachment 174368 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=174368&action=review
> > > 
> > > >>> Source/WebCore/css/SelectorChecker.cpp:1042
> > > >>> +            if (element && (element->isFormControlElement() || element->hasTagName(optionTag) || element->hasTagName(optgroupTag) || element->isLink()))
> > > >> 
> > > >> Won't this match the parent image of the AREA rather than the AREA element? That is any IMG element with a USEMAP property.
> > > > 
> > > > Ah, thanks! We have to check tagName.
> > > 
> > > By the way this implementation is completely wrong :-(
> > 
> > The more I think about it, I guess you should skip handling AREA elements to begin with. It seems they haven't thought it through in the standard. AREAs at least in WebKit do not even have their own render object, and will therefore not have their own style either, even if we manage to set a specific style for them in ElementRareData, it will not be used, because AREAs are not rendered.
> 
> Hmm... the reason why AREA is not rendered is just AREA has display: none.
> Actually my test renders AREA.

That is part of it. The reason they have display:none is that they are supposed to be used together with MAP and IMG elements, AREA elements represent different shapes (often non-rectangular) in an image. These shapes are where they respond to mouse-clicks, but since the image is rendered by the IMG element, the AREA elements are not supposed to render anything themselves. 

So your test-case works, but you are not using AREA in the role it is meant to be used. If I was a web-designer I would expect something like area:enabled or area:hover to be able to show me the areas in the images that they respond to, but we do not support having special styles for these areas.

[Shinya Kawanaka]

> That is part of it. The reason they have display:none is that they are supposed to be used together with MAP and IMG elements, AREA elements represent different shapes (often non-rectangular) in an image. These shapes are where they respond to mouse-clicks, but since the image is rendered by the IMG element, the AREA elements are not supposed to render anything themselves. 
> 
> So your test-case works, but you are not using AREA in the role it is meant to be used. If I was a web-designer I would expect something like area:enabled or area:hover to be able to show me the areas in the images that they respond to, but we do not support having special styles for these areas.

I understand I was doing some non-expected thing. I was just obeying the spec.
Actually, I don't have any strong insistence that :enabled should match AREA, though. Basically it's meaningless for now. So, anyway, I'll remove AREA support from this patch.

[Shinya Kawanaka]

Created attachment 174577 [details]
Patch

[Kent Tamura]

Comment on attachment 174577 [details]
Patch

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

> Source/WebCore/css/SelectorChecker.cpp:1046
> +                if (element->hasTagName(aTag))
> +                    return element->isLink();

We shouldn't add hasTagName checks in CSS code any more, should have Element::matchesEnabledPseudoClass virtual function instead.