Some applications truncates the last closing parenthesis in perf dashboard URL
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 24 May 2016 00:24:57 +0000 (00:24 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 24 May 2016 00:24:57 +0000 (00:24 +0000)
https://bugs.webkit.org/show_bug.cgi?id=157976

Reviewed by Tim Horton.

Unfortunately, different applications use different heuristics to determine the end of each URL. Two trailing
parentheses, for example, is just fine in Radar as well as Apple Mail if the URL is short enough. Using other
characters such as ] and } wouldn't work either because they would be %-escaped. At that point, we might as well
as %-escape everything.

Work around the bug by parsing the URL as if it had one extra ')' if the parsing had failed. Also shorten the charts
page's URL by avoid emitting "-null" for each pane when the pane doesn't have a currently selected point or selection.
This improves the odds of the entire URL being recognized by various applications.

We could, in theory, implement some sort of a URL shorter but that can wait until when we support real user accounts.

* public/v3/pages/chart-pane.js:
(ChartPane.prototype.serializeState): Don't serialize the selection or the current point if nothing is selected.
* public/v3/pages/page-router.js:
(PageRouter.prototype._deserializeHashQueryValue): Try parsing the value again with one extra ] at the end if
the JSON parsing had failed.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@201307 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/public/v3/pages/chart-pane.js
Websites/perf.webkit.org/public/v3/pages/page-router.js

index 40d45c4..4a47280 100644 (file)
@@ -1,3 +1,27 @@
+2016-05-23  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Some applications truncates the last closing parenthesis in perf dashboard URL
+        https://bugs.webkit.org/show_bug.cgi?id=157976
+
+        Reviewed by Tim Horton.
+
+        Unfortunately, different applications use different heuristics to determine the end of each URL. Two trailing
+        parentheses, for example, is just fine in Radar as well as Apple Mail if the URL is short enough. Using other
+        characters such as ] and } wouldn't work either because they would be %-escaped. At that point, we might as well
+        as %-escape everything.
+
+        Work around the bug by parsing the URL as if it had one extra ')' if the parsing had failed. Also shorten the charts
+        page's URL by avoid emitting "-null" for each pane when the pane doesn't have a currently selected point or selection.
+        This improves the odds of the entire URL being recognized by various applications.
+
+        We could, in theory, implement some sort of a URL shorter but that can wait until when we support real user accounts.
+
+        * public/v3/pages/chart-pane.js:
+        (ChartPane.prototype.serializeState): Don't serialize the selection or the current point if nothing is selected.
+        * public/v3/pages/page-router.js:
+        (PageRouter.prototype._deserializeHashQueryValue): Try parsing the value again with one extra ] at the end if
+        the JSON parsing had failed.
+
 2016-05-18  Ryosuke Niwa  <rniwa@webkit.org>
 
         Perf dashboard "Add pane" should list first by test, then by machine
index ba043f0..8f39a01 100644 (file)
@@ -15,13 +15,15 @@ class ChartPane extends ChartPaneBase {
 
     serializeState()
     {
-        var selection = this._mainChart ? this._mainChart.currentSelection() : null;
-        var point = this._mainChart ? this._mainChart.currentPoint() : null;
-        return [
-            this._platformId,
-            this._metricId,
-            selection || (point && this._mainChartIndicatorWasLocked ? point.id : null),
-        ];
+        var state = [this._platformId, this._metricId];
+        if (this._mainChart) {
+            var selection = this._mainChart.currentSelection();
+            if (selection)
+                state[2] = selection;
+            else if (this._mainChartIndicatorWasLocked)
+                state[2] = this._mainChart.currentPoint().id;
+        }
+        return state;
     }
 
     updateFromSerializedState(state, isOpen)
index 6d04ef1..cc782d5 100644 (file)
@@ -140,9 +140,24 @@ class PageRouter {
 
     _deserializeHashQueryValue(value)
     {
+        var json = value.replace(/\(/g, '[').replace(/\)/g, ']').replace(/-/g, ',');
         try {
-            return JSON.parse(value.replace(/\(/g, '[').replace(/\)/g, ']').replace(/-/g, ','));
+            return JSON.parse(json);
         } catch (error) {
+
+            // Some applications don't linkify two consecutive closing parentheses: )).
+            // Try fixing adding one extra parenthesis to see if that works.
+            function count(regex)
+            {
+                var match = json.match(regex);
+                return match ? match.length : 0;
+            }
+            var missingClosingBrackets = count(/\[/g) - count(/\]/g);
+            var fix = new Array(missingClosingBrackets).fill(']').join('');
+            try {
+                return JSON.parse(json + fix);
+            } catch (newError) { }
+
             return value;
         }
     }