Bug 152657 - Web Inspector: Add context menu items to CSS selectors to auto-generate pseudo selectors
Summary: Web Inspector: Add context menu items to CSS selectors to auto-generate pseud...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-01-02 17:35 PST by Devin Rousso
Modified: 2016-01-04 19:03 PST (History)
8 users (show)

See Also:


Attachments
Patch (12.71 KB, patch)
2016-01-02 18:11 PST, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2016-01-02 17:35:37 PST
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.
Comment 1 Radar WebKit Bug Importer 2016-01-02 17:35:56 PST
<rdar://problem/24032328>
Comment 2 Devin Rousso 2016-01-02 18:11:35 PST
Created attachment 268123 [details]
Patch
Comment 3 Timothy Hatcher 2016-01-04 10:47:12 PST
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?
Comment 4 Devin Rousso 2016-01-04 10:50:55 PST
(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.
Comment 5 WebKit Commit Bot 2016-01-04 12:40:11 PST
Comment on attachment 268123 [details]
Patch

Clearing flags on attachment: 268123

Committed r194547: <http://trac.webkit.org/changeset/194547>
Comment 6 WebKit Commit Bot 2016-01-04 12:40:16 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 Joseph Pecoraro 2016-01-04 18:56:51 PST
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.
Comment 8 Devin Rousso 2016-01-04 19:03:04 PST
(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).