[webkit-reviews] review granted: [Bug 204924] Fix inspector/css test assertions after r253158 : [Attachment 384967] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Dec 5 15:20:52 PST 2019


Devin Rousso <drousso at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 204924: Fix inspector/css test assertions after r253158
https://bugs.webkit.org/show_bug.cgi?id=204924

Attachment 384967: Patch

https://bugs.webkit.org/attachment.cgi?id=384967&action=review




--- Comment #3 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 384967
  --> https://bugs.webkit.org/attachment.cgi?id=384967
Patch

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

r=me, with one additional fix

> Source/WebCore/ChangeLog:8
> +	   No new tests (OOPS!).

Oops.

> Source/WebInspectorUI/UserInterface/Controllers/CSSManager.js:754
> +    Highlight: "highlight",

In addition to this, I think the only thing that's necessary for the Web
Inspector frontend to handle this is adding the following to
`WI.CSSManager.displayNameForPseudoId`:
```
    case CSSManager.PseudoSelectorNames.Highlight:
	return WI.unlocalizedString("::highlight");
```
Without this, the Web Inspector frontend would log an error to the console. 
I'd rather fix both the backend `ASSERT` and any frontend requirements at the
same time.

It's not necessary to add it to the switch-case inside the `if
(!InspectorBackend.Enum.CSS.PseudoId)` as that's only used for compatibility
purposes when inspecting older webpages, meaning that path won't be taken when
inspecting a page that has `PseudoId::Highlight` defined.


More information about the webkit-reviews mailing list