Invert the new incognito tab Spyman orientation and location for RTL....

Invert the new incognito tab Spyman orientation and location for RTL.

Also, make the in-page spy man face the text. Thus he will face the opposite direction of the titlebar spy guy.

BUG=22872
BUG=22935
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=27231

[Evan Stade, 2009-09-24 20:22:03]

See http://www/~estade/no_crawl/spyguy.png for examples in Hebrew and English on
Linux. Ignore the bookmark bar's appearance and the rendering in Hebrew for the
bullet points, those are both separate issues.

[arv (Not doing code reviews), 2009-09-24 23:06:08]

http://codereview.chromium.org/238005/diff/1/2
File chrome/browser/resources/incognito_tab.html (right):

http://codereview.chromium.org/238005/diff/1/2#newcode9
Line 9: html[dir=ltr] .icon {
No need for the attribute selector here:

html .icon {

http://codereview.chromium.org/238005/diff/1/2#newcode12
Line 12: -webkit-transform: matrix(-1, 0, 0, 1, 0, 0);
scale(-1, 1) seems easier to grok

http://codereview.chromium.org/238005/diff/1/2#newcode14
Line 14: html[dir=rtl] .icon {
Please quote your attribute values

html[dir="rtl"] .icon {

http://codereview.chromium.org/238005/diff/1/2#newcode16
Line 16: margin:0px 6px 3px 0px;
This order is not the opposite of the ltr case. You should use:

margin: 0 3px 0 6px;

Please remove units on zeros.

[Evan Stade, 2009-09-24 23:16:13]

(css noob)

http://codereview.chromium.org/238005/diff/1/2
File chrome/browser/resources/incognito_tab.html (right):

http://codereview.chromium.org/238005/diff/1/2#newcode9
Line 9: html[dir=ltr] .icon {
On 2009/09/24 23:06:08, arv wrote:
> No need for the attribute selector here:
> 
> html .icon {
> 
> 

i assume the margins and float would be overridden below but if we don't qualify
this then wouldn't it cause the transform to apply in RTL mode as well?

http://codereview.chromium.org/238005/diff/1/2#newcode12
Line 12: -webkit-transform: matrix(-1, 0, 0, 1, 0, 0);
On 2009/09/24 23:06:08, arv wrote:
> scale(-1, 1) seems easier to grok

ah. of course.

http://codereview.chromium.org/238005/diff/1/2#newcode14
Line 14: html[dir=rtl] .icon {
On 2009/09/24 23:06:08, arv wrote:
> Please quote your attribute values
> 
> html[dir="rtl"] .icon {

will do.

http://codereview.chromium.org/238005/diff/1/2#newcode16
Line 16: margin:0px 6px 3px 0px;
On 2009/09/24 23:06:08, arv wrote:
> This order is not the opposite of the ltr case. You should use:
> 
> margin: 0 3px 0 6px;
> 
> Please remove units on zeros.

will do

[arv (Not doing code reviews), 2009-09-24 23:53:39]

On Thu, Sep 24, 2009 at 16:16, <estade@chromium.org> wrote:

> (css noob)
>
>
> http://codereview.chromium.org/238005/diff/1/2
> File chrome/browser/resources/incognito_tab.html (right):
>
> http://codereview.chromium.org/238005/diff/1/2#newcode9
> Line 9: html[dir=ltr] .icon {
> On 2009/09/24 23:06:08, arv wrote:
>
>> No need for the attribute selector here:
>>
>
>  html .icon {
>>
>
>
>
> i assume the margins and float would be overridden below but if we don't
> qualify this then wouldn't it cause the transform to apply in RTL mode
> as well?


You're right of course


>
>
> http://codereview.chromium.org/238005/diff/1/2#newcode12
> Line 12: -webkit-transform: matrix(-1, 0, 0, 1, 0, 0);
> On 2009/09/24 23:06:08, arv wrote:
>
>> scale(-1, 1) seems easier to grok
>>
>
> ah. of course.
>
> http://codereview.chromium.org/238005/diff/1/2#newcode14
> Line 14: html[dir=rtl] .icon {
> On 2009/09/24 23:06:08, arv wrote:
>
>> Please quote your attribute values
>>
>
>  html[dir="rtl"] .icon {
>>
>
> will do.
>
> http://codereview.chromium.org/238005/diff/1/2#newcode16
> Line 16: margin:0px 6px 3px 0px;
> On 2009/09/24 23:06:08, arv wrote:
>
>> This order is not the opposite of the ltr case. You should use:
>>
>
>  margin: 0 3px 0 6px;
>>
>
>  Please remove units on zeros.
>>
>
> will do
>
>
> http://codereview.chromium.org/238005
>



-- 
erik

[Evan Stade, 2009-09-25 00:57:39]

updated

[arv (Not doing code reviews), 2009-09-25 03:15:30]

http://codereview.chromium.org/238005/diff/5001/2002
File chrome/browser/resources/incognito_tab.html (right):

http://codereview.chromium.org/238005/diff/5001/2002#newcode12
Line 12: -webkit-transform: scale(-1, 1);
Isn't the flipping supposed to be done in rtl?

[Evan Stade, 2009-09-25 03:57:34]

http://codereview.chromium.org/238005/diff/5001/2002
File chrome/browser/resources/incognito_tab.html (right):

http://codereview.chromium.org/238005/diff/5001/2002#newcode12
Line 12: -webkit-transform: scale(-1, 1);
On 2009/09/25 03:15:30, arv wrote:
> Isn't the flipping supposed to be done in rtl?

This part is a change to LTR. See the screenshots, compare to current new
incognito tab.

[arv (Not doing code reviews), 2009-09-25 16:08:18]

LGTM

On Thu, Sep 24, 2009 at 20:57, <estade@chromium.org> wrote:

>
> http://codereview.chromium.org/238005/diff/5001/2002
> File chrome/browser/resources/incognito_tab.html (right):
>
> http://codereview.chromium.org/238005/diff/5001/2002#newcode12
> Line 12: -webkit-transform: scale(-1, 1);
> On 2009/09/25 03:15:30, arv wrote:
>
>> Isn't the flipping supposed to be done in rtl?
>>
>
> This part is a change to LTR. See the screenshots, compare to current
> new incognito tab.
>
>
> http://codereview.chromium.org/238005
>



-- 
erik