Bug 90261

Summary:

Move platform/qt/fast/js/navigator-language.html to fast/js.

Product:

WebKit

Reporter:

Kihong Kwon <kihong.kwon>

Component:

WebKit API

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

andersca, gyuyoung.kim, rakuco, rniwa, vimff0, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

89639

Bug Blocks:

Attachments:

Description	Flags
Patch	none
Patch	rniwa: review-, rniwa: commit-queue-
patch	none

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.