Bug 199594 – Web Inspector: Elements: always show all navigation bar items, but disable those that wouldn't work

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Devin Rousso]

Instead of only showing icons for functionality that is supported by the inspected target, we should always show all of the icons and just disable the ones that wouldn't work.

[Devin Rousso]

Created attachment 373934 [details]
Patch

[Ross Kirsling]

Comment on attachment 373934 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/DOMTreeContentView.js:36
> +        this._compositingBordersButtonNavigationItem.enabled = InspectorBackend.domains.Page;

Did you mean to drop the existing condition?

> this._compositingBordersButtonNavigationItem.enabled = InspectorBackend.domains.Page && !!InspectorBackend.domains.Page.getCompositingBordersVisible;

[Devin Rousso]

Comment on attachment 373934 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/DOMTreeContentView.js:36
>> +        this._compositingBordersButtonNavigationItem.enabled = InspectorBackend.domains.Page;
> 
> Did you mean to drop the existing condition?

No, but I do need to add a `!!`.  The idea is that we only want to enable this button if the target has a `PageAgent`.  `Page.getCompositingBordersVisible` exists for all supported iOS inspection targets, so we don't need a more specific compatibility check than that.

[Ross Kirsling]

(In reply to Devin Rousso from comment #3)
> Comment on attachment 373934 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=373934&action=review
> 
> >> Source/WebInspectorUI/UserInterface/Views/DOMTreeContentView.js:36
> >> +        this._compositingBordersButtonNavigationItem.enabled = InspectorBackend.domains.Page;
> > 
> > Did you mean to drop the existing condition?
> 
> No, but I do need to add a `!!`.  The idea is that we only want to enable
> this button if the target has a `PageAgent`. 
> `Page.getCompositingBordersVisible` exists for all supported iOS inspection
> targets, so we don't need a more specific compatibility check than that.

Ahh, I'm now seeing the correspondence with your // COMPATIBILITY comments. :)

[Ross Kirsling]

Comment on attachment 373934 [details]
Patch

r=me with that tiny update then :)

[Devin Rousso]

Created attachment 374037 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 374037 [details]
Patch

Clearing flags on attachment: 374037

Committed r247405: <https://trac.webkit.org/changeset/247405>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/53032515>