Bug 163705 – Web Inspector: Improve Quick Open sorting algorithm

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Nikita Vasilyev]

Created attachment 292126 [details]
[Image] Bug

Steps:
1. Press Command-Shift-O.
2. Type "Color"

Expected:
The first item in the list is should start with "Color", e.g. "Color.js".

Actual:
The first item in the list is "ConsoleCommandResultMessage.js".

[Radar WebKit Bug Importer]

<rdar://problem/28858237>

[Devin Rousso]

Created attachment 292290 [details]
Patch

[Build Bot]

Comment on attachment 292290 [details]
Patch

Attachment 292290 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2334488

New failing tests:
inspector/unit-tests/resource-query-controller.html

[Build Bot]

Created attachment 292299 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 292290 [details]
Patch

Attachment 292290 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2334536

New failing tests:
inspector/unit-tests/resource-query-controller.html

[Build Bot]

Created attachment 292303 [details]
Archive of layout-test-results from ews113 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 292290 [details]
Patch

Attachment 292290 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2334610

New failing tests:
inspector/unit-tests/resource-query-controller.html

[Build Bot]

Created attachment 292304 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Joseph Pecoraro]

Comment on attachment 292290 [details]
Patch

r- Based on the test failures I'm not sure this is the right approach. Likewise, we should have a new test for the new case this is addressing!

I think in the original example, this should behave the same regardless if the input is "Color" or "color".

    "color" should prefer "Color.js" over "ConsoleCommandResultMessage.js"

I think this means we should instead adjust our point system so that consecutive characters eventually have an advantage over special character matches.

Some different approaches that look like it would satisfy this case:

    (1) weight higher consecutive characters at the start of a candidate name
        - would help with "color" for "Color.js" vs "CxxOxxLxxOxxR.js"
    (2) weight higher consecutive characters at the start of a special character
        - would help with "color" in "Color.js" and "HighlightColor.js" vs "CxxOxxLxxOxxR.js"
        - would help with "cview" in "ContentView.js" vs "CxxVxxIxxExxW.js"
    (3) weight higher any longer sequence of consecutive characters no matter if it starts at a special character
        - would help with "hshot" in "HeapSnapshot.js" vs "HxxSxxHxxOxxT.js"

I'm in favor of (2). (1) is a special case of (2) that maybe we should consider boosting points for as well. (3) is a pattern I don't think users will think about and use. (2) is definitely what I think of when I type in these kinds of dialogs.

[Devin Rousso]

Created attachment 292569 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 292569 [details]
Patch

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

r=me!

> Source/WebInspectorUI/UserInterface/Models/ResourceQueryResult.js:91
> +                this._rank += consecutiveWeight * getMultiplier(consecutiveMatchStart) * (match.index - consecutiveMatchStart.index);

Did I follow this correctly? (Matching "cview" in "ContentView.js")

   "c" : rank += 10*5  = 50  (+50)
       : rank -= 0     = 50  (+0)
   "v" : rank += 10*5  = 100 (+50)
       : rank -= 7*5   = 65  (-35)
   "i" : rank += 10*1  = 75  (+10)
       : rank += 5*5*1 = 100 (+25)
       : rank -= 8*1   =  92 (-8)
   "e" : rank += 10*1  = 102 (+10)
       : rank += 5*5*2 = 152 (+50)
       : rank -= 9*1   = 143 (-9)
   "w" : rank += 10*1  = 153 (+10)
       : rank += 5*5*3 = 228 (+75)
       : rank -= 10*1  = 218 (-10)

If I did, then spread out special characters are 50 each.

    50 (50)
    100 (50 + 50)
    150 (50 + 50 + 50)
    200 (50 + 50 + 50 + 50)

So a 3-4 letter consecutive character sequence starts overtaking 3-4 special characters.

    25 (25)
    75 (25 + 50)
    150 (25 + 50 + 75)
    300 (25 + 50 + 75 + 150)

I think the subtraction at the end should make 3 special < 3 consecutive in the majority of cases, which I kind of like because consecutive 3 likely means a vowel and 3 special likely means no vowel.

I like this!

> Source/WebInspectorUI/UserInterface/Models/ResourceQueryResult.js:98
> +            // the beginning of the string are ranked higher.  Increase the amound subtracted if

Typo: "amound" => "amount".

[Devin Rousso]

Created attachment 292652 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 292652 [details]
Patch

Clearing flags on attachment: 292652

Committed r207782: <http://trac.webkit.org/changeset/207782>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.