Bug 152657 – Web Inspector: Add context menu items to CSS selectors to auto-generate pseudo selectors

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

[Devin Rousso]

I often find that having to add ":hover" or "::before" to a style can be very tedious, so a simple context menu item to "Create ::before Rule" would be very helpful.  Clicking this item would generate a new rule with the same selector, except each selector (if they are comma separated) is appended with "::before" and the text of this rule would be set to "content: normal;" as per the spec.  There would be menu items for ":active", ":focus", ":hover", ":visited" (only for links), "::before", and "::after" in that order.  Obviously, if any of the selectors already contains the item, don't show it in the context menu.

[Radar WebKit Bug Importer]

<rdar://problem/24032328>

[Devin Rousso]

Created attachment 268123 [details]
Patch

[Timothy Hatcher]

Comment on attachment 268123 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.js:440
> +            InspectorFrontendHost.copyText(this._style.generateCSSRuleString());

We have had issues with lexical bound this in arrow functions. Was this (and other arrow functions) working?

[Devin Rousso]

(In reply to comment #3)
> We have had issues with lexical bound this in arrow functions. Was this (and
> other arrow functions) working?

Interestingly enough, I haven't had any issues with context menu arrow functions. Not really sure why... It is very temperamental and "sometimes" breaks for certain functions and is just fine for others.

[WebKit Commit Bot]

Comment on attachment 268123 [details]
Patch

Clearing flags on attachment: 268123

Committed r194547: <http://trac.webkit.org/changeset/194547>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Joseph Pecoraro]

Comment on attachment 268123 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/VisualStyleSelectorTreeItem.js:161
> +        // Only used one colon temporarily since single-colon pseudo elements are valid CSS.
> +        if (WebInspector.CSSStyleManager.PseudoElementNames.some((className) => this.representedObject.selectorText.includes(":" + className)))
> +            return;

It would be really nice to de-duplicate this code somehow. We have lots of duplication of ContextMenu duplications all over over the place, so this is not new. But coming up with a good scheme to de-dup this would be great! I totally see one of these getting out of sync with the other.

[Devin Rousso]

(In reply to comment #7)
> It would be really nice to de-duplicate this code somehow. We have lots of
> duplication of ContextMenu duplications all over over the place, so this is
> not new. But coming up with a good scheme to de-dup this would be great! I
> totally see one of these getting out of sync with the other.

Yeah I was trying to think of a way to do just that, but each of them have small minute differences that I couldn't think of anything simple.  My one idea was to make some sort of static generator which takes in the event and a list of unique id's (like one for "Copy Rule" and one for "Add :hover Rule") and returns the context menu item, but it would require a centralization of these types of context menu events (especially hard since the two cases here differ in the way that they refer to their respective CSSStyleDeclaration). I'll try to work it out a bit more tho (let me know if what I wrote above is just complete gibberish).