Bug 163705

Summary: Web Inspector: Improve Quick Open sorting algorithm
Product: WebKit Reporter: Nikita Vasilyev <nvasilyev>
Component: Web InspectorAssignee: Devin Rousso <drousso>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, buildbot, commit-queue, drousso, joepeck, mattbaker, nvasilyev, rniwa, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
[Image] Bug
none
Patch
none
Archive of layout-test-results from ews103 for mac-yosemite
none
Archive of layout-test-results from ews113 for mac-yosemite
none
Archive of layout-test-results from ews104 for mac-yosemite-wk2
none
Patch
joepeck: review+
Patch none

Description Nikita Vasilyev 2016-10-19 16:22:56 PDT
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".
Comment 1 Radar WebKit Bug Importer 2016-10-19 16:23:14 PDT
<rdar://problem/28858237>
Comment 2 Devin Rousso 2016-10-20 17:12:47 PDT
Created attachment 292290 [details]
Patch
Comment 3 Build Bot 2016-10-20 18:04:09 PDT
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
Comment 4 Build Bot 2016-10-20 18:04:12 PDT
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
Comment 5 Build Bot 2016-10-20 18:19:57 PDT
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
Comment 6 Build Bot 2016-10-20 18:19:59 PDT
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
Comment 7 Build Bot 2016-10-20 18:21:07 PDT
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
Comment 8 Build Bot 2016-10-20 18:21:10 PDT
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
Comment 9 Joseph Pecoraro 2016-10-21 14:02:41 PDT
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.
Comment 10 Devin Rousso 2016-10-23 17:56:45 PDT
Created attachment 292569 [details]
Patch
Comment 11 Joseph Pecoraro 2016-10-24 12:45:35 PDT
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".
Comment 12 Devin Rousso 2016-10-24 14:16:28 PDT
Created attachment 292652 [details]
Patch
Comment 13 WebKit Commit Bot 2016-10-24 14:51:01 PDT
Comment on attachment 292652 [details]
Patch

Clearing flags on attachment: 292652

Committed r207782: <http://trac.webkit.org/changeset/207782>
Comment 14 WebKit Commit Bot 2016-10-24 14:51:08 PDT
All reviewed patches have been landed.  Closing bug.