Bug 204902 – Add CSS parser support for the highlight pseudoelement

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Simon Fraser (smfr)]

Add CSS parser support for the highlight pseudoelement

[Simon Fraser (smfr)]

Created attachment 384924 [details]
Patch

[Simon Fraser (smfr)]

https://trac.webkit.org/changeset/253158/webkit

[Radar WebKit Bug Importer]

<rdar://problem/57670974>

[Darin Adler]

Comment on attachment 384924 [details]
Patch

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

> Source/WebCore/rendering/style/RenderStyleConstants.h:85
> -enum class PseudoId : uint8_t {
> +enum class PseudoId : uint16_t {

What’s the rationale for this change?

[Simon Fraser (smfr)]

(In reply to Darin Adler from comment #4)
> Comment on attachment 384924 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=384924&action=review
> 
> > Source/WebCore/rendering/style/RenderStyleConstants.h:85
> > -enum class PseudoId : uint8_t {
> > +enum class PseudoId : uint16_t {
> 
> What’s the rationale for this change?

Compiler was telling me that uint8_t wasn't big enough at the:
PublicPseudoIdMask = ((1 << FirstInternalPseudoId) - 1) & ~((1 << FirstPublicPseudoId) - 1)
line, but maybe I need to fix that bitmasking instead.

[Darin Adler]

Comment on attachment 384924 [details]
Patch

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

>>> Source/WebCore/rendering/style/RenderStyleConstants.h:85
>>> +enum class PseudoId : uint16_t {
>> 
>> What’s the rationale for this change?
> 
> Compiler was telling me that uint8_t wasn't big enough at the:
> PublicPseudoIdMask = ((1 << FirstInternalPseudoId) - 1) & ~((1 << FirstPublicPseudoId) - 1)
> line, but maybe I need to fix that bitmasking instead.

Looks to me like the issue is that PublicPseudoIdMask should not be a PseudoId value. It’s mask for use when these PseudoId values are used as bit numbers, not one of the PseudoId values. So we should move it out of the enum class.

[Simon Fraser (smfr)]

Yeah I'll do that.