Bug 149608

Summary:

Dashboard metrics has JS errors from toggle button patch

Product:

WebKit

Reporter:

Dean Johnson <dean_johnson>

Component:

Tools / Tests

Assignee:

Dean Johnson <dean_johnson>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none

Dean Johnson

Reported 2015-09-28 15:46:36 PDT

The Dashboard metrics page has JS errors that were introduced with https://bugs.webkit.org/show_bug.cgi?id=148403. This patch fixes them and also moves some of the identical logic from dashboard/Scripts/Main.js and dashboard/Scripts/MetricsMain.js to dashboard/Scripts/Settings.js.

Attachments
Patch (10.54 KB, patch) 2015-09-28 15:51 PDT, Dean Johnson	no flags	Details Formatted Diff Diff
Patch (11.68 KB, patch) 2015-09-28 16:12 PDT, Dean Johnson	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Dean Johnson

Comment 1 2015-09-28 15:51:39 PDT

Created attachment 262025 [details] Patch

Alexey Proskuryakov

Comment 2 2015-09-28 15:55:56 PDT

Comment on attachment 262025 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=262025&action=review > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/MetricsMain.js:346 > + var settingsWrapper = document.createElement("div"); Eventually we should refactor this to have less copy/pasted code. > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/Settings.js:91 > + if (platformName) > + return platformName.substr(0, platformName.indexOf("-")); > + return '' Coding style nit: we prefer early return, so this should be if (!platformName) return '' return platformName.substr(0, platformName.indexOf("-"));

Dean Johnson

Comment 3 2015-09-28 16:12:40 PDT

Created attachment 262026 [details] Patch

Dean Johnson

Comment 4 2015-09-28 16:13:21 PDT

(In reply to comment #2) > Comment on attachment 262025 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=262025&action=review > > > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/MetricsMain.js:346 > > + var settingsWrapper = document.createElement("div"); > > Eventually we should refactor this to have less copy/pasted code. > Agreed. > > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/Settings.js:91 > > + if (platformName) > > + return platformName.substr(0, platformName.indexOf("-")); > > + return '' > > Coding style nit: we prefer early return, so this should be > > if (!platformName) > return '' > return platformName.substr(0, platformName.indexOf("-")); Fixed in latest patch.

WebKit Commit Bot

Comment 5 2015-09-28 21:31:41 PDT

Comment on attachment 262026 [details] Patch Clearing flags on attachment: 262026 Committed r190303: <http://trac.webkit.org/changeset/190303>

WebKit Commit Bot

Comment 6 2015-09-28 21:31:43 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.