Bug 115014 - Web Inspector: Add number to list from remote web inspector.
Summary: Web Inspector: Add number to list from remote web inspector.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-22 22:46 PDT by Seokju Kwon
Modified: 2013-04-24 03:49 PDT (History)
5 users (show)

See Also:


Attachments
Inspectable Webview list (45.08 KB, image/png)
2013-04-22 22:47 PDT, Seokju Kwon
no flags Details
Patch (1.72 KB, patch)
2013-04-22 22:49 PDT, Seokju Kwon
no flags Details | Formatted Diff | Diff
Patch (2.07 KB, patch)
2013-04-23 19:41 PDT, Seokju Kwon
no flags Details | Formatted Diff | Diff
Patch (2.10 KB, patch)
2013-04-24 00:51 PDT, Seokju Kwon
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Seokju Kwon 2013-04-22 22:46:52 PDT
It is difficult to make a distinction if pages have a long url or title.
(Please find the attached file)
Comment 1 Seokju Kwon 2013-04-22 22:47:41 PDT
Created attachment 199150 [details]
Inspectable Webview list
Comment 2 Seokju Kwon 2013-04-22 22:49:23 PDT
Created attachment 199151 [details]
Patch
Comment 3 Seokju Kwon 2013-04-23 00:27:09 PDT
CC'ed : WebKit2 Owner(Benjamin Poulain)
Comment 4 Seokju Kwon 2013-04-23 00:27:17 PDT
CC'ed : WebKit2 Owner(Benjamin Poulain)
Comment 5 Joseph Pecoraro 2013-04-23 18:29:25 PDT
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.
Comment 6 Joseph Pecoraro 2013-04-23 18:30:29 PDT
> 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.
Comment 7 Seokju Kwon 2013-04-23 19:40:00 PDT
JoePeck : Thanks. As mentioned on irc, I will use <ol> tag.
Comment 8 Seokju Kwon 2013-04-23 19:41:53 PDT
Created attachment 199379 [details]
Patch
Comment 9 Benjamin Poulain 2013-04-24 00:33:52 PDT
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.
Comment 10 Seokju Kwon 2013-04-24 00:51:50 PDT
Created attachment 199401 [details]
Patch
Comment 11 WebKit Commit Bot 2013-04-24 03:49:32 PDT
Comment on attachment 199401 [details]
Patch

Clearing flags on attachment: 199401

Committed r149025: <http://trac.webkit.org/changeset/149025>
Comment 12 WebKit Commit Bot 2013-04-24 03:49:34 PDT
All reviewed patches have been landed.  Closing bug.