Bug 182326 - Should choose the best match during 'route' if there are multiple matches.
Summary: Should choose the best match during 'route' if there are multiple matches.
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-01-31 01:13 PST by dewei_zhu
Modified: 2019-06-16 21:12 PDT (History)
2 users (show)

See Also:

Attachments
Patch (6.82 KB, patch)
2018-01-31 01:23 PST, dewei_zhu 		no flags Details | Formatted Diff | Diff
Patch (9.45 KB, patch)
2018-01-31 14:22 PST, dewei_zhu 		rniwa: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description dewei_zhu 2018-01-31 01:13:47 PST 
Should chose the best match during 'route' when there are multiple matches.
Comment 1 dewei_zhu 2018-01-31 01:23:49 PST 
Created attachment 332750 [details]
Patch
Comment 2 Ryosuke Niwa 2018-01-31 11:34:40 PST 
Comment on attachment 332750 [details]
Patch

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

r- due to the lack of testing.

> Websites/perf.webkit.org/public/v3/pages/page-router.js:43
> +                } else if (hashUrl.startsWith(routeName) && hashUrl.charAt(routeName.length) == '/' && bestMatchingRouteName.length < routeName.length) {

What's hashUrl.charAt(routeName.length) == '/' about?
Comment 3 dewei_zhu 2018-01-31 13:33:15 PST 
Comment on attachment 332750 [details]
Patch

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

>> Websites/perf.webkit.org/public/v3/pages/page-router.js:43
>> +                } else if (hashUrl.startsWith(routeName) && hashUrl.charAt(routeName.length) == '/' && bestMatchingRouteName.length < routeName.length) {
> 
> What's hashUrl.charAt(routeName.length) == '/' about?

This check exists before. I would assume we want '/apple/xxxx' matches '/apple', and do not want it matches '/applepile/...'
Comment 4 dewei_zhu 2018-01-31 14:22:31 PST 
Created attachment 332802 [details]
Patch
Comment 5 Ryosuke Niwa 2018-01-31 15:00:10 PST 
Comment on attachment 332802 [details]
Patch

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

> Websites/perf.webkit.org/public/v3/pages/page-router.js:32
> +            let bestMatchingRouteName = '';

Initialize this to null instead.