Bug 49692 - Rebaseline server: add loupe for image diffs
Summary: Rebaseline server: add loupe for image diffs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Mihai Parparita
URL:
Keywords:
Depends on:
Blocks: 47761
  Show dependency treegraph
 
Reported: 2010-11-17 15:27 PST by Mihai Parparita
Modified: 2010-11-17 16:42 PST (History)
1 user (show)

See Also:


Attachments
Patch (14.25 KB, patch)
2010-11-17 15:29 PST, Mihai Parparita
no flags Details | Formatted Diff | Diff
Patch (14.22 KB, patch)
2010-11-17 16:34 PST, Mihai Parparita
tony: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mihai Parparita 2010-11-17 15:27:25 PST
Rebaseline server: add loupe for image diffs
Comment 1 Mihai Parparita 2010-11-17 15:29:27 PST
Created attachment 74163 [details]
Patch
Comment 2 Tony Chang 2010-11-17 16:26:09 PST
Comment on attachment 74163 [details]
Patch

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

This sounds awesome.  I'll have to try the rebaseline tool after this lands.  r- for naming style, otherwise, this looks good.

> WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/loupe.js:37
> +    this.node_ = $('loupe');
> +    this.currentCornerX_ = -1;
> +    this.currentCornerY_ = -1;

There's no real webkit style for JS objects.  Ojan and I discussed briefly and he suggested just using _node and _currentCornerX for the following reasons:
1) JS outside of google seems to use prefix _ more than suffix _.
2) It's similar to PEP8 and JS seems more like python than C++.

> WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/loupe.js:55
> +Loupe.prototype.handleOutputClick_ = function(event)

To go with the above recommendation, this would be _handleOutputClick.
Comment 3 Mihai Parparita 2010-11-17 16:34:33 PST
Created attachment 74176 [details]
Patch
Comment 4 Mihai Parparita 2010-11-17 16:34:57 PST
Switched to leading underscore in new version of the patch.
Comment 5 Mihai Parparita 2010-11-17 16:42:36 PST
Committed r72262: <http://trac.webkit.org/changeset/72262>