Bug 89362 - [garden-o-matic] Builder names without underscores cause incorrect BuildSelector behavior
Summary: [garden-o-matic] Builder names without underscores cause incorrect BuildSelec...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zan Dobersek
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-18 10:56 PDT by Zan Dobersek
Modified: 2012-06-18 12:04 PDT (History)
4 users (show)

See Also:


Attachments
Patch (2.31 KB, patch)
2012-06-18 11:06 PDT, Zan Dobersek
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2012-06-18 10:56:51 PDT
Builder names without underscores cause the BuildSelector object to not store the containers in the proper way, displaying all the containers instead of just the one the selected tab is linked with.

An example of this is when viewing results for the Apple builders - the container id and hash that's used in the associated tab are just encoded as URI components (the result of the current platform's resultsDirectoryNameFromBuilderName method), which doesn't work. Chromium doesn't have such problems as the builders' results directories are already replacing whitespace, braces and dots with underscores.

JQuery documentation[1] suggests something similar.

I think it's safe to convert whitespace, braces and dots to underscores for every builder name in ui.results.BuilderSelector.init and use that to link the container with the associated tab.

[1] - http://docs.jquery.com/UI/Tabs#Ajax_mode
Comment 1 Simon Fraser (smfr) 2012-06-18 11:05:27 PDT
I'm not quite following how to see the actual bug in the garden-o-matic UI.
Comment 2 Zan Dobersek 2012-06-18 11:06:03 PDT
Created attachment 148130 [details]
Patch
Comment 3 Simon Fraser (smfr) 2012-06-18 11:10:49 PDT
Comment on attachment 148130 [details]
Patch

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

> Tools/ChangeLog:12
> +        (.):

Remove this.

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/scripts/ui/results.js:370
> +            var builderHash = builderName.replace(/[ .()]/g, '_');

This looks very similar to config.resultsDirectoryNameFromBuilderName() for chromium. Maybe put a function in base and call it in both places.
Comment 4 Zan Dobersek 2012-06-18 12:04:01 PDT
(In reply to comment #3)
> > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/scripts/ui/results.js:370
> > +            var builderHash = builderName.replace(/[ .()]/g, '_');
> 
> This looks very similar to config.resultsDirectoryNameFromBuilderName() for chromium. Maybe put a function in base and call it in both places.

Created base.underscoredBuilderName.

Committed http://trac.webkit.org/changeset/120614.