Bug 115014 – Web Inspector: Add number to list from remote web inspector.

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

[Seokju Kwon]

It is difficult to make a distinction if pages have a long url or title.
(Please find the attached file)

[Seokju Kwon]

Created attachment 199150 [details]
Inspectable Webview list

[Seokju Kwon]

Created attachment 199151 [details]
Patch

[Seokju Kwon]

CC'ed : WebKit2 Owner(Benjamin Poulain)

[Seokju Kwon]

CC'ed : WebKit2 Owner(Benjamin Poulain)

[Joseph Pecoraro]

Comment on attachment 199151 [details]
Patch

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

> Source/WebKit2/UIProcess/InspectorServer/front-end/inspectorPageIndex.html:16
> +                link.appendChild(document.createTextNode("[" + i + "] " + title + (url ? (" [" + url + "]") : "" )));

The for loop is currently "for (var i in pages)". However, "pages" in an array. The for loop should be a regular for loop, not a for..in loop.

    for (var i = 0; i < pages.length; ++i) { ... }

Are the #s important? It looks like the backend generates this array from HashMap iterators. I don't think that guarantees the order of items. Should the items be sorted somewhere here so the numbers don't change seemingly randomly if the Hashmap changes its size?

This change is simple and harmless however, so if it helps it looks good to me.

[Joseph Pecoraro]

> Are the #s important? It looks like the backend generates this array from HashMap iterators. I don't think that guarantees the order of items. Should the items be sorted somewhere here so the numbers don't change seemingly randomly if the Hashmap changes its size?

I forgot to mention here, but I did on IRC, an <ol> would probably work too.

[Seokju Kwon]

JoePeck : Thanks. As mentioned on irc, I will use <ol> tag.

[Seokju Kwon]

Created attachment 199379 [details]
Patch

[Benjamin Poulain]

Comment on attachment 199379 [details]
Patch

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

> Source/WebKit2/UIProcess/InspectorServer/front-end/inspectorPageIndex.html:22
> +                var page = document.createElement("li");
> +                page.appendChild(link);
> +                pageList.appendChild(page);

page -> pageListItem or something like that.

The variable "page" should only be use for "pages[i]" in this context.

[Seokju Kwon]

Created attachment 199401 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 199401 [details]
Patch

Clearing flags on attachment: 199401

Committed r149025: <http://trac.webkit.org/changeset/149025>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.