Bug 90261 - Move platform/qt/fast/js/navigator-language.html to fast/js.
Summary: Move platform/qt/fast/js/navigator-language.html to fast/js.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 89639
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-29 02:16 PDT by Kihong Kwon
Modified: 2012-07-01 22:22 PDT (History)
6 users (show)

See Also:


Attachments
Patch (4.36 KB, patch)
2012-06-29 05:31 PDT, Kihong Kwon
no flags Details | Formatted Diff | Diff
Patch (4.21 KB, patch)
2012-06-29 18:44 PDT, Kihong Kwon
rniwa: review-
rniwa: commit-queue-
Details | Formatted Diff | Diff
patch (6.96 KB, patch)
2012-07-01 19:33 PDT, Kihong Kwon
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kihong Kwon 2012-06-29 02:16:38 PDT
Efl needs a test case for navigator.language.
This test case is modified for all ports.
And I would like to move to fast/js.
I checked this on the qt, gtk, mac and efl.
Comment 1 Alexey Proskuryakov 2012-06-29 04:57:42 PDT
Fast/js is where JavaScript-only tests live, i.e. ones that don't depend on browser DOM. So, both current and suggested places are wrong.
Comment 2 Kihong Kwon 2012-06-29 05:24:10 PDT
(In reply to comment #1)
> Fast/js is where JavaScript-only tests live, i.e. ones that don't depend on browser DOM. So, both current and suggested places are wrong.

Originally this test case is in the fast/js before it was moved to platform/qt/fast/js. (bug 83710)
It was my reason for moving to fast/js.
Could you tell me where is proper position for this?
Comment 3 Kihong Kwon 2012-06-29 05:31:51 PDT
Created attachment 150154 [details]
Patch
Comment 4 Kihong Kwon 2012-06-29 05:36:17 PDT
Anders, could you check this patch please?
Because I rolled back your change.(bug 83710)
Comment 5 Ryosuke Niwa 2012-06-29 12:28:58 PDT
Comment on attachment 150154 [details]
Patch

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

> LayoutTests/fast/js/navigator-language.html:10
> +    <head>
> +        <script src="../../fast/js/resources/js-test-pre.js"></script>
> +    </head>
> +    <body>
> +        <script>
> +            description(
> +                "<pre>Check return value of navigator.language."
> +            );

Why are you indenting elements and scripts like this? We don't normally do this.
Also, please use svn mv.

> LayoutTests/fast/js/navigator-language.html:18
> +				shouldBe("language", "'en'");

We use 4-space indentation, not tabs.
Comment 6 Kihong Kwon 2012-06-29 18:44:27 PDT
Created attachment 150288 [details]
Patch
Comment 7 Kihong Kwon 2012-06-29 18:45:24 PDT
(In reply to comment #5)
> (From update of attachment 150154 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=150154&action=review
> 
> > LayoutTests/fast/js/navigator-language.html:10
> > +    <head>
> > +        <script src="../../fast/js/resources/js-test-pre.js"></script>
> > +    </head>
> > +    <body>
> > +        <script>
> > +            description(
> > +                "<pre>Check return value of navigator.language."
> > +            );
> 
> Why are you indenting elements and scripts like this? We don't normally do this.
> Also, please use svn mv.
> 
> > LayoutTests/fast/js/navigator-language.html:18
> > +				shouldBe("language", "'en'");
> 
> We use 4-space indentation, not tabs.

It was my mistake.
They were fixed.
Comment 8 Ryosuke Niwa 2012-06-29 18:56:10 PDT
Comment on attachment 150288 [details]
Patch

Again, you didn't use svn mv.
Comment 9 Kihong Kwon 2012-06-29 19:57:25 PDT
(In reply to comment #8)
> (From update of attachment 150288 [details])
> Again, you didn't use svn mv.

But, I need to change test case and expected result.
After I change the file, git makes them to new file.
Do you have a way for keeping it 'renamed'?
Comment 10 Ryosuke Niwa 2012-06-29 20:06:45 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > (From update of attachment 150288 [details] [details])
> > Again, you didn't use svn mv.
> 
> But, I need to change test case and expected result.
> After I change the file, git makes them to new file.
> Do you have a way for keeping it 'renamed'?

Don't use git for this. Use a svn checkout for patches like this.
Comment 11 Kihong Kwon 2012-07-01 19:33:27 PDT
Created attachment 150354 [details]
patch
Comment 12 WebKit Review Bot 2012-07-01 22:22:13 PDT
Comment on attachment 150354 [details]
patch

Clearing flags on attachment: 150354

Committed r121656: <http://trac.webkit.org/changeset/121656>
Comment 13 WebKit Review Bot 2012-07-01 22:22:18 PDT
All reviewed patches have been landed.  Closing bug.