[webkit-reviews] review granted: [Bug 200425] Web Inspector: Sources: provide a way to create an arbitrary Inspector Style Sheet : [Attachment 375608] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Aug 15 14:16:08 PDT 2019

Joseph Pecoraro <joepeck at webkit.org> has granted Devin Rousso
<drousso at apple.com>'s request for review:
Bug 200425: Web Inspector: Sources: provide a way to create an arbitrary
Inspector Style Sheet

Attachment 375608: Patch


--- Comment #8 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 375608
  --> https://bugs.webkit.org/attachment.cgi?id=375608

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


I still think the (+) button should be in the left corner. Have you played with
that? Can we file a follow-up to experiment with that and see if it feels
better? That matches the rest of the system, and the consistently of knowing
"the bottom left corner will always have this button" is a conceptual win.

> Source/WebInspectorUI/UserInterface/Base/URLUtilities.js:244
> +    if (options.allowDirectoryAsName && url !== "about:blank" &&
(!displayName || urlComponents.path.endsWith(displayName + "/")))

This makes me think we probably have others ("about:srcdoc" comes to mind).

Also `javascript:`, `mailto:`, `about:`, `blob:` and other common prefixes.

Most of what I've been doing recently is checking if the url starts with
"http:" to be more proactive then reactive. Does that make sense here?

We should really cover `WI.displayNameForURL` with tests.

> +	       let addInspectorStyleSheetCheckbox = (parent, frame) => {

`parent` sounds like a node or tree element. Perhaps `menu` would be clearer.

> LayoutTests/inspector/unit-tests/url-utilities-expected.txt:252
> +FAIL: origin should be: 'http://example.com'
> +    Expected: "http://example.com"
> +    Actual: null

Whats up with these failures?

More information about the webkit-reviews mailing list