Title: [201307] trunk/Websites/perf.webkit.org
Revision
201307
Author
[email protected]
Date
2016-05-23 17:24:57 -0700 (Mon, 23 May 2016)

Log Message

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.

Modified Paths

Diff

Modified: trunk/Websites/perf.webkit.org/ChangeLog (201306 => 201307)


--- trunk/Websites/perf.webkit.org/ChangeLog	2016-05-24 00:22:53 UTC (rev 201306)
+++ trunk/Websites/perf.webkit.org/ChangeLog	2016-05-24 00:24:57 UTC (rev 201307)
@@ -1,3 +1,27 @@
+2016-05-23  Ryosuke Niwa  <[email protected]>
+
+        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  <[email protected]>
 
         Perf dashboard "Add pane" should list first by test, then by machine

Modified: trunk/Websites/perf.webkit.org/public/v3/pages/chart-pane.js (201306 => 201307)


--- trunk/Websites/perf.webkit.org/public/v3/pages/chart-pane.js	2016-05-24 00:22:53 UTC (rev 201306)
+++ trunk/Websites/perf.webkit.org/public/v3/pages/chart-pane.js	2016-05-24 00:24:57 UTC (rev 201307)
@@ -15,13 +15,15 @@
 
     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)

Modified: trunk/Websites/perf.webkit.org/public/v3/pages/page-router.js (201306 => 201307)


--- trunk/Websites/perf.webkit.org/public/v3/pages/page-router.js	2016-05-24 00:22:53 UTC (rev 201306)
+++ trunk/Websites/perf.webkit.org/public/v3/pages/page-router.js	2016-05-24 00:24:57 UTC (rev 201307)
@@ -140,9 +140,24 @@
 
     _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;
         }
     }
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to