Title: [227938] trunk/Websites/perf.webkit.org
- Revision
- 227938
- Author
- [email protected]
- Date
- 2018-01-31 15:41:37 -0800 (Wed, 31 Jan 2018)
Log Message
Should chose the best match during 'route' if there are multiple matches.
https://bugs.webkit.org/show_bug.cgi?id=182326
Reviewed by Ryosuke Niwa.
r227749 made a change that 'analysisCategoryPage' will be added before 'analysisTaskPage'.
As route names for both pages starts with 'analysis', whichever added first will be chosen.
For a route like 'analysis/task/1'. As a result, 'analysisCategoryPage' will be chosen and
this is not expected behavior. Adding the logic on the cases when route name does not extact
match the route name, always choose the longest mathcing route name.
Also modernized the code of 'page-router.js' to use const & let instead of var.
Added a browser test to guard against this bug.
* browser-tests/index.html: Import 'page-router-tests.js'.
* browser-tests/page-router-tests.js: Added unit test to guard against this bug.
* public/v3/pages/page-router.js:
(PageRouter.prototype.route): Added logic to find best matching in the case of inexact match.
(PageRouter.prototype.pageDidOpen):
(PageRouter.prototype._updateURLState):
(PageRouter.prototype._serializeToHash):
(PageRouter.prototype._deserializeFromHash):
(PageRouter.prototype._serializeHashQueryValue):
(PageRouter.prototype._deserializeHashQueryValue):
(PageRouter.prototype._countOccurrences):
(PageRouter):
Modified Paths
Added Paths
Diff
Modified: trunk/Websites/perf.webkit.org/ChangeLog (227937 => 227938)
--- trunk/Websites/perf.webkit.org/ChangeLog 2018-01-31 23:32:36 UTC (rev 227937)
+++ trunk/Websites/perf.webkit.org/ChangeLog 2018-01-31 23:41:37 UTC (rev 227938)
@@ -1,3 +1,33 @@
+2018-01-31 Dewei Zhu <[email protected]>
+
+ Should chose the best match during 'route' if there are multiple matches.
+ https://bugs.webkit.org/show_bug.cgi?id=182326
+
+ Reviewed by Ryosuke Niwa.
+
+ r227749 made a change that 'analysisCategoryPage' will be added before 'analysisTaskPage'.
+ As route names for both pages starts with 'analysis', whichever added first will be chosen.
+ For a route like 'analysis/task/1'. As a result, 'analysisCategoryPage' will be chosen and
+ this is not expected behavior. Adding the logic on the cases when route name does not extact
+ match the route name, always choose the longest mathcing route name.
+
+ Also modernized the code of 'page-router.js' to use const & let instead of var.
+
+ Added a browser test to guard against this bug.
+
+ * browser-tests/index.html: Import 'page-router-tests.js'.
+ * browser-tests/page-router-tests.js: Added unit test to guard against this bug.
+ * public/v3/pages/page-router.js:
+ (PageRouter.prototype.route): Added logic to find best matching in the case of inexact match.
+ (PageRouter.prototype.pageDidOpen):
+ (PageRouter.prototype._updateURLState):
+ (PageRouter.prototype._serializeToHash):
+ (PageRouter.prototype._deserializeFromHash):
+ (PageRouter.prototype._serializeHashQueryValue):
+ (PageRouter.prototype._deserializeHashQueryValue):
+ (PageRouter.prototype._countOccurrences):
+ (PageRouter):
+
2018-01-29 Dewei Zhu <[email protected]>
Should fetch owner commits in build-requests-fetcher.
Modified: trunk/Websites/perf.webkit.org/browser-tests/index.html (227937 => 227938)
--- trunk/Websites/perf.webkit.org/browser-tests/index.html 2018-01-31 23:32:36 UTC (rev 227937)
+++ trunk/Websites/perf.webkit.org/browser-tests/index.html 2018-01-31 23:41:37 UTC (rev 227938)
@@ -17,6 +17,7 @@
<div id="mocha"></div>
<script src=""
<script src=""
+<script src=""
<script src=""
<script src=""
<script src=""
Added: trunk/Websites/perf.webkit.org/browser-tests/page-router-tests.js (0 => 227938)
--- trunk/Websites/perf.webkit.org/browser-tests/page-router-tests.js (rev 0)
+++ trunk/Websites/perf.webkit.org/browser-tests/page-router-tests.js 2018-01-31 23:41:37 UTC (rev 227938)
@@ -0,0 +1,36 @@
+
+describe('PageRouter', () => {
+ describe('route', () => {
+ it('should choose the longest match', async () => {
+ const context = new BrowsingContext();
+ const [Page, PageRouter, ComponentBase] = await context.importScripts(
+ ['instrumentation.js', 'components/base.js', 'pages/page.js', 'pages/page-router.js'],
+ 'Page', 'PageRouter', 'ComponentBase');
+
+ let someRenderCount = 0;
+ class SomePage extends Page {
+ constructor() { super('some page'); }
+ render() { someRenderCount++; }
+ routeName() { return 'page'; }
+ };
+
+ let anotherRenderCount = 0;
+ class AnotherPage extends Page {
+ constructor() { super('another page'); }
+ render() { anotherRenderCount++; }
+ routeName() { return 'page/another'; }
+ };
+
+ const router = new PageRouter;
+ const somePage = new SomePage;
+ const anotherPage = new AnotherPage;
+ router.addPage(somePage);
+ router.addPage(anotherPage);
+ context.global.location.hash = '#/page/another/1';
+ router.route();
+ await waitForComponentsToRender(context);
+ expect(someRenderCount).to.be(0);
+ expect(anotherRenderCount).to.be(1);
+ });
+ });
+});
\ No newline at end of file
Modified: trunk/Websites/perf.webkit.org/public/v3/pages/page-router.js (227937 => 227938)
--- trunk/Websites/perf.webkit.org/public/v3/pages/page-router.js 2018-01-31 23:32:36 UTC (rev 227937)
+++ trunk/Websites/perf.webkit.org/public/v3/pages/page-router.js 2018-01-31 23:41:37 UTC (rev 227938)
@@ -25,23 +25,31 @@
route()
{
- var destinationPage = this._defaultPage;
- var parsed = this._deserializeFromHash(location.hash);
+ let destinationPage = this._defaultPage;
+ const parsed = this._deserializeFromHash(location.hash);
if (parsed.route) {
- var hashUrl = parsed.route;
- var queryIndex = hashUrl.indexOf('?');
+ let hashUrl = parsed.route;
+ let bestMatchingRouteName = null;
+ const queryIndex = hashUrl.indexOf('?');
if (queryIndex >= 0)
hashUrl = hashUrl.substring(0, queryIndex);
- for (var page of this._pages) {
- var routeName = page.routeName();
- if (routeName == hashUrl
- || (hashUrl.startsWith(routeName) && hashUrl.charAt(routeName.length) == '/')) {
- parsed.state.remainingRoute = hashUrl.substring(routeName.length + 1);
+ for (const page of this._pages) {
+ const routeName = page.routeName();
+
+ if (routeName == hashUrl) {
+ bestMatchingRouteName = routeName;
destinationPage = page;
break;
+ } else if (hashUrl.startsWith(routeName) && hashUrl.charAt(routeName.length) == '/'
+ && (!bestMatchingRouteName || bestMatchingRouteName.length < routeName.length)) {
+ bestMatchingRouteName = routeName;
+ destinationPage = page;
}
}
+
+ if (bestMatchingRouteName)
+ parsed.state.remainingRoute = hashUrl.substring(bestMatchingRouteName.length + 1);
}
if (!destinationPage)
@@ -60,7 +68,7 @@
pageDidOpen(page)
{
console.assert(page instanceof Page);
- var pageDidChange = this._currentPage != page;
+ const pageDidChange = this._currentPage != page;
this._currentPage = page;
if (pageDidChange)
this.scheduleUrlStateUpdate();
@@ -82,7 +90,7 @@
{
this._historyTimer = null;
console.assert(this._currentPage);
- var currentPage = this._currentPage;
+ const currentPage = this._currentPage;
this._hash = this._serializeToHash(currentPage.routeName(), currentPage.serializeState());
location.hash = this._hash;
}
@@ -97,10 +105,10 @@
_serializeToHash(route, state)
{
- var params = [];
- for (var key in state)
+ const params = [];
+ for (const key in state)
params.push(key + '=' + this._serializeHashQueryValue(state[key]));
- var query = params.length ? ('?' + params.join('&')) : '';
+ const query = params.length ? ('?' + params.join('&')) : '';
return `#/${route}${query}`;
}
@@ -111,13 +119,13 @@
hash = unescape(hash); // For Firefox.
- var queryIndex = hash.indexOf('?');
- var route;
- var state = {};
+ const queryIndex = hash.indexOf('?');
+ let route;
+ const state = {};
if (queryIndex >= 0) {
route = hash.substring(2, queryIndex);
- for (var part of hash.substring(queryIndex + 1).split('&')) {
- var keyValuePair = part.split('=');
+ for (const part of hash.substring(queryIndex + 1).split('&')) {
+ const keyValuePair = part.split('=');
state[keyValuePair[0]] = this._deserializeHashQueryValue(keyValuePair[1]);
}
} else
@@ -129,8 +137,8 @@
_serializeHashQueryValue(value)
{
if (value instanceof Array) {
- var serializedItems = [];
- for (var item of value)
+ const serializedItems = [];
+ for (const item of value)
serializedItems.push(this._serializeHashQueryValue(item));
return '(' + serializedItems.join('-') + ')';
}
@@ -143,11 +151,11 @@
_deserializeHashQueryValue(value)
{
if (value.charAt(0) == '(') {
- var nestingLevel = 0;
- var end = 0;
- var start = 1;
- var result = [];
- for (var character of value) {
+ let nestingLevel = 0;
+ let end = 0;
+ let start = 1;
+ const result = [];
+ for (const character of value) {
if (character == '(')
nestingLevel++;
else if (character == ')') {
@@ -176,7 +184,7 @@
_countOccurrences(string, regex)
{
- var match = string.match(regex);
+ const match = string.match(regex);
return match ? match.length : 0;
}
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
