Bug 39994 - Geolocation LayoutTests should make more careful use of LayoutTestController
Summary: Geolocation LayoutTests should make more careful use of LayoutTestController
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-01 06:49 PDT by Steve Block
Modified: 2010-06-04 02:33 PDT (History)
3 users (show)

See Also:


Attachments
Patch (25.79 KB, patch)
2010-06-01 07:01 PDT, Steve Block
no flags Details | Formatted Diff | Diff
Patch (26.02 KB, patch)
2010-06-01 13:36 PDT, Steve Block
no flags Details | Formatted Diff | Diff
Patch (29.30 KB, patch)
2010-06-04 02:21 PDT, Steve Block
jorlow: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Block 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.
Comment 1 Steve Block 2010-06-01 07:01:02 PDT
Created attachment 57539 [details]
Patch
Comment 2 Steve Block 2010-06-01 13:36:37 PDT
Created attachment 57589 [details]
Patch
Comment 3 Dumitru Daniliuc 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)?
Comment 4 Steve Block 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?
Comment 5 Dumitru Daniliuc 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. :)
Comment 6 Steve Block 2010-06-04 02:21:29 PDT
Created attachment 57856 [details]
Patch
Comment 7 Jeremy Orlow 2010-06-04 02:24:36 PDT
Comment on attachment 57856 [details]
Patch

r=me
Comment 8 Steve Block 2010-06-04 02:33:35 PDT
Committed r60667: <http://trac.webkit.org/changeset/60667>