Bug 39994

Summary: Geolocation LayoutTests should make more careful use of LayoutTestController
Product: WebKit Reporter: Steve Block <steveblock>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: dumi, jorlow, steveblock
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch jorlow: review+

Steve Block
Reported 2010-06-01 06:49:17 PDT
The Geolocation LayoutTests should guard calls to LayoutTestController methods with 'if (window.layoutTestController)' and should make it clear if the test can not pass without LayoutTestController.
Attachments
Patch (25.79 KB, patch)
2010-06-01 07:01 PDT, Steve Block 		no flags
DetailsFormatted DiffDiff
Patch (26.02 KB, patch)
2010-06-01 13:36 PDT, Steve Block 		no flags
DetailsFormatted DiffDiff
Patch (29.30 KB, patch)
2010-06-04 02:21 PDT, Steve Block 		jorlow: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Steve Block
Comment 1 2010-06-01 07:01:02 PDT
Created attachment 57539 [details] Patch
Steve Block
Comment 2 2010-06-01 13:36:37 PDT
Created attachment 57589 [details] Patch
Dumitru Daniliuc
Comment 3 2010-06-03 15:13:17 PDT
LGTM, but i have an unrelated question about the .js files in fast/dom/Geolocation/script-tests/. lots of them seem to have code that looks like this: }, function(e) { error = e shouldBe('error.code', 'mockCode'); shouldBe('error.message', 'mockMessage'); ... shouldn't there be a ';' after 'error = e' (at least from a style point of view)?
Steve Block
Comment 4 2010-06-03 15:35:26 PDT
> shouldn't there be a ';' after 'error = e' (at least from a style point of view)? Yes, good catch. I can fix this on landing if that's OK with you?
Dumitru Daniliuc
Comment 5 2010-06-03 15:46:14 PDT
That's fine with me. Please go through every .js file as quite a few of them seem to have this issue. Oh, and you're still going to need an official r+ from somebody else since I'm not a reviewer... sorry. :)
Steve Block
Comment 6 2010-06-04 02:21:29 PDT
Created attachment 57856 [details] Patch
Jeremy Orlow
Comment 7 2010-06-04 02:24:36 PDT
Comment on attachment 57856 [details] Patch r=me
Steve Block
Comment 8 2010-06-04 02:33:35 PDT
Committed r60667: <http://trac.webkit.org/changeset/60667>
Note You need to log in before you can comment on or make changes to this bug.