Summary: | Web Inspector: Add number to list from remote web inspector. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Seokju Kwon <seokju> | ||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | benjamin, commit-queue, graouts, joepeck, timothy | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Seokju Kwon
2013-04-22 22:46:52 PDT
Created attachment 199150 [details]
Inspectable Webview list
Created attachment 199151 [details]
Patch
CC'ed : WebKit2 Owner(Benjamin Poulain) CC'ed : WebKit2 Owner(Benjamin Poulain) 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. > 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.
JoePeck : Thanks. As mentioned on irc, I will use <ol> tag. Created attachment 199379 [details]
Patch
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. Created attachment 199401 [details]
Patch
Comment on attachment 199401 [details] Patch Clearing flags on attachment: 199401 Committed r149025: <http://trac.webkit.org/changeset/149025> All reviewed patches have been landed. Closing bug. |