Bug 27673 – Inspector: Tab Through Attributes When Editing

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Joseph Pecoraro]

Nice feature that is in Firebug that most developers will expect.

Here is the work in progress, patch will be applied in a minute:
http://screencast.com/t/wAsMntnR1

[Joseph Pecoraro]

Created attachment 33486 [details]
Tab Through Attributes

Also fixes (and improves the fix) for:
https://bugs.webkit.org/show_bug.cgi?id=27526

[Joseph Pecoraro]

*** Bug 27526 has been marked as a duplicate of this bug. ***

[Joseph Pecoraro]

Created attachment 33487 [details]
Tab Through Attributes

I had forgotten the try/catch block from the earlier bug =).  All better now.

[Joseph Pecoraro]

After the first patch is applied this next patch (attached in a secon) will allow for tabbing though CSS Properties!  This took absolutely forever to get perfect but I finally got it working exactly how I wanted it:
http://screencast.com/t/MMWHpaaK

[Joseph Pecoraro]

Created attachment 33503 [details]
Tab Through CSS Properties (Needs First Patch)

See Previous Comment for a video.

[Joseph Pecoraro]

Created attachment 33504 [details]
Tab Through CSS Properties (Needs First Patch)

Removed Excess Whitespace.

[Joseph Pecoraro]

Rik` (Anthony Ricaud) from IRC  just mentioned:

Need a way to go to the next CSS declaration:
https://bugs.webkit.org/show_bug.cgi?id=21629

Adding new CSS style properties to HTML nodes using Web Inspector:
https://bugs.webkit.org/show_bug.cgi?id=18224

I will mark 21629 as a duplicate of this one, but 18224 still exists and can probably be solved quite similarly (or like its done with element attributes).

[Joseph Pecoraro]

*** This bug has been marked as a duplicate of bug 21629 ***

[Joseph Pecoraro]

Wrong way around.

[Joseph Pecoraro]

Created attachment 33510 [details]
Tab Through CSS Properties (Needs First Patch)

I found a way to eliminate firing the "dblclick" event in one case and just directly start editing (thanks to bug 18224's patch from way back when).  This case is for the new property and there is no text to select. I might even eventually do the same for the condition (tabbing to a non-new attribute), but for right now its fine, because it gets to specifically pick something to have selected when starting to edit.

[Timothy Hatcher]

Comment on attachment 33487 [details]
Tab Through Attributes


> +                attributeElements[i].nextSibling.nextSibling.dispatchEvent(event); // "=" textNode, then the "webkit-html-attribute-value" node

All looks fine except this. I fear this is too fragile. It would be better to loop through the next siblings until you found "webkit-html-attribute-value".

[Timothy Hatcher]

Comment on attachment 33510 [details]
Tab Through CSS Properties (Needs First Patch)

> +                if (elem.className === "name") {

This would be safer as "elem.hasStyleClass("name")" so if we add more to the className later it still works.

> +            // Find the attribute we were supposed to start editing in this section
> +            var list = section.propertiesTreeOutline._childrenListNode.childNodes;
> +            for (var i = 0, len = list.length; i < len; ++i) {
> +                var listItem = list[i];
> +                var spans = listItem.getElementsByTagName("span");
> +                if (spans[0].textContent === nameOfPropretyToEdit) {
> +                    var event = document.createEvent("MouseEvents");
> +                    event.initMouseEvent("dblclick", true, true);
> +                    spans[1].dispatchEvent(event); // the value
> +                    return;
> +                }
> +            }
> +        }

Over all these patches look good, but I think they are too low level and fragile at this point. They are working with the DOM too much when they should just be working with the high-level TreeElements and TreeOutlines. This would require additions to StylePropertyTreeElement like "editName" or "editValue" that would start editing on one of the parts without the need for a fake event and getElementsByTagName or using private properties like _childrenListNode.

[Timothy Hatcher]

Comment on attachment 33487 [details]
Tab Through Attributes


> +        var attributeElements = this.listItemElement.getElementsByClassName("webkit-html-attribute-name");
> +        for (var i = 0, len = attributeElements.length; i < len; ++i) {
> +            if (attributeElements[i].textContent === attributeName) {
> +                var event = document.createEvent("MouseEvents");
> +                event.initMouseEvent("dblclick", true, true);
> +                attributeElements[i].nextSibling.nextSibling.dispatchEvent(event); // "=" textNode, then the "webkit-html-attribute-value" node
> +                break;
> +            }
> +        }
> +    },

This should be done without a fake event by asking the ElementsTreeElement to start editing on a specific attribute's value or name.

[Joseph Pecoraro]

Created attachment 33511 [details]
Tab Through Attributes/Properties (BOTH)

Both patches together.

[Joseph Pecoraro]

Created attachment 33512 [details]
Tab Through Attributes/Properties (BOTH)

Sorry about the spam, I had forgotten a Changelog.

[Joseph Pecoraro]

(In reply to comment #11)
> All looks fine except this. I fear this is too fragile. It would be better to
> loop through the next siblings until you found "webkit-html-attribute-value".

Done. Ultimately Element Attributes should become more like CSS Properties in the StylesSidebar.  Right now the attributes are generated HTML and I am forced to use DOM Traversal functions.  In the StylesSidebar they are actual TreeElements and I can work on a higher level.

> This would be safer as "elem.hasStyleClass("name")" so if we add more to the
> className later it still works.

No Longer Needed (see next).

> Over all these patches look good, but I think they are too low level and
> fragile at this point. They are working with the DOM too much when they should
> just be working with the high-level TreeElements and TreeOutlines.

Done. Once I understood the structure of the higher-level nodes it became much easier (TreeOutline, Pane, TreeSection, TreeElement).(In reply to comment #13)

> This should be done without a fake event by asking the ElementsTreeElement to
> start editing on a specific attribute's value or name.

Done. The only remaining fake-event is with Element Attributes of which I cannot find a workaround for until its refactored.

---

There are a lot of edge cases in ElementAttributes due to the fact that it is just loosely floating HTML.  And there are even some edge cases in CSS Properties (such as editing an attribute and then pushing tab silently wipes things!).  I believe I handle everything correctly and I've tested quite a bit.

[Timothy Hatcher]

Landed in r46429.