Bug 199555 – Web Inspector: Layers: Uncaught Exception: Request with id = 2 failed. {"code":-32601,"message":"'Page' domain was not found","data":[{"code":-32601,"message":"'Page' domain was not found"}]}

• Component: Web Inspector
• Status: RESOLVED FIXED

[Devin Rousso]

# STEPS TO REPRODUCE:
1. inspect any page
2. enable the Layers tab
3. go to the Layers tab
4. close Web Inspector
5. open Web Inspector (make sure the Layers tab is initially selected)

[Devin Rousso]

Created attachment 373579 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 373579 [details]
Patch

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

r- to see an updated patch. The changes in general seem the right direction.

> Source/WebInspectorUI/UserInterface/Views/DOMTreeContentView.js:41
> +        if (InspectorBackend.domains.Page && InspectorBackend.domains.Page.showPaintRects) {

There is a peculiar difference between these two features after the change:

  • Compositing Borders button shows even if the backend doesn't support it, but shows disabled
  • Paint Rects button shows only if the backend supports it

I'd rather they be consistent. Seem they should both only check Page domain or they should both check Page domain + feature. Any reason to differ?

> Source/WebInspectorUI/UserInterface/Views/DOMTreeContentView.js:46
> +            this._paintFlashingButtonNavigationItem.enabled = !!InspectorBackend.domains.Page.setShowPaintRects;
> +            this._paintFlashingButtonNavigationItem.activated = InspectorBackend.domains.Page.setShowPaintRects && WI.settings.showPaintRects.value;

If we decide to keep the change in the condition above, then this code can be improved.

  • We wouldn't need the `enabled` assignment since it would always be true.
  • Likewise this doesn't need to check `InspectorBackend.domains.Page.setShowPaintRects` since it is guaranteed to be true.

> Source/WebInspectorUI/UserInterface/Views/DOMTreeContentView.js:599
> +               target.PageAgent.setCompositingBordersVisible(activated);

Style: Leading whitespace is off (for all of these target.PageAgent calls after a target.PageAgent check).

> Source/WebInspectorUI/UserInterface/Views/DOMTreeContentView.js:743
> +               target.PageAgent.setCompositingBordersVisible(activated);

Wrong API call! This should be PageAgent.ssetShowRulers

[Devin Rousso]

Created attachment 373674 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 373674 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Views/DOMTreeContentView.js:639
> +               target.PageAgent.setShowPaintRects(activated);

Whitespace is still wrong here.

> Source/WebInspectorUI/UserInterface/Views/DOMTreeContentView.js:660
> +               target.PageAgent.setEmulatedMedia(mediaType);

Whitespace is still wrong here.

> Source/WebInspectorUI/UserInterface/Views/DOMTreeContentView.js:741
> +        for (let target of WI.targets) {

Style: Put a newline before this?

> Source/WebInspectorUI/UserInterface/Views/Layers3DContentView.js:398
> +        for (let target of WI.targets) {

Style: Put a newline before this?

> Source/WebInspectorUI/UserInterface/Views/Layers3DContentView.js:400
> +               target.PageAgent.setShowPaintRects(activated);

Whitespace is still wrong here.

> Source/WebInspectorUI/UserInterface/Views/Layers3DContentView.js:433
> +        for (let target of WI.targets) {

Style: Put a newline before this?

[Devin Rousso]

Created attachment 373930 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 373930 [details]
Patch

Clearing flags on attachment: 373930

Committed r247356: <https://trac.webkit.org/changeset/247356>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/52963131>