1061003 - Add New Rule won't work in non-english locales

Status: VERIFIED FIXED

(DevTools :: Inspector, defect)

Description

[Dave Camp (:dcamp)]

Add new rule uses:
  if (rule.selectorText === "element") {

to determine the element rule.  But selectorText is internationalized in that case.

It should use:

rule.domRule.type === ELEMENT_STYLE

I think.

Comment 1

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: 1061003.patch

https://tbpl.mozilla.org/?tree=Try&rev=5ef6ab997891

Comment 2

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/fae611216e7f

Comment 3

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fae611216e7f

Comment 4

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 8482875 [details] [diff] [review]
1061003.patch

Approval Request Comment
[Feature/regressing bug #]: 1061003
[User impact if declined]: Add New Rule won't work in non-english locales
[Describe test coverage new/current, TBPL]: Feature is already available on beta. Changes to support non-english locales is only checking the type property of the rule instead of the string.
[Risks and why]:  None
[String/UUID change made/needed]: None

Comment 5

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8482875 [details] [diff] [review]
1061003.patch

(In reply to Gabriel Luong (:gl) from comment #4)
> [Feature/regressing bug #]: 1061003

Bug 1061003 is this bug. When was this issue introduced into the code base?

> [Describe test coverage new/current, TBPL]: Feature is already available on
> beta. Changes to support non-english locales is only checking the type
> property of the rule instead of the string.

I'm confused. Do you mean that this fix has already landed on beta (in another bug)? If not, should this fix be uplifted to beta as well? 

ni on Gabriel about whether to uplift to beta.

Aurora+

Comment 6

[Gabriel Luong [:gl] (ΦωΦ)]

(In reply to Lawrence Mandel [:lmandel] from comment #5)
> Comment on attachment 8482875 [details] [diff] [review]
> 1061003.patch
> 
> (In reply to Gabriel Luong (:gl) from comment #4)
> > [Feature/regressing bug #]: 1061003
> 
> Bug 1061003 is this bug. When was this issue introduced into the code base?
> 

Ah sorry, the original feature was introduced in Bug 966895.

> > [Describe test coverage new/current, TBPL]: Feature is already available on
> > beta. Changes to support non-english locales is only checking the type
> > property of the rule instead of the string.
> 
> I'm confused. Do you mean that this fix has already landed on beta (in
> another bug)? If not, should this fix be uplifted to beta as well? 
> 
> ni on Gabriel about whether to uplift to beta.
> 
> Aurora+

The original feature introduced in Bug 966895 should be in Firefox 33 (currently in beta). I think it would be good to uplift to beta as well if possible.

Comment 7

[Lawrence Mandel [:lmandel] (use needinfo)]

I see. I misunderstood your comment as this fix was already on beta. Please request beta approval and let's get this uplifted.

Comment 8

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 8482875 [details] [diff] [review]
1061003.patch

Approval Request Comment
[Feature/regressing bug #]: 1061003
[User impact if declined]: Add New Rule won't work in non-english locales
[Describe test coverage new/current, TBPL]: Green try, and bug fix has been in m-c for about a week with no reported problems.
[Risks and why]: None
[String/UUID change made/needed]: None

Comment 9

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8482875 [details] [diff] [review]
1061003.patch

Beta+

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/314a3069165d
https://hg.mozilla.org/releases/mozilla-beta/rev/bacdfedd7241

Comment 11

[Ada [:adalucinet]]

Reproduced with Firefox 33 beta 2 de build: the Add rule option via Inspector/Rules is not working.
Verified as fixed on Mac OS X 10.9.4, a new rule is added with:
- Firefox 33 beta 4 (Build ID: 20140915204924) de, ar, fr, it and ko builds
- latest Aurora 34.0a2 (Build ID: 20140916004001) es-ES, da, gl and ja builds
- latest Nightly 35.0a1 (Build ID: 20140916030204) hu, lt, nb-No and ru builds

During testing, I've encountered some issues:
[1] On ko, gl and lt builds the Shortcut key for Inspector is not displayed (via Tools -> Web Developer); at a second attempt it appears and is the same as the one for Toggle Tools;
[2] 'Add rule' is not translated on ar nor nb-NO builds;
 
Should I file a bug on this matter? Or those are known issues?

Comment 12

[Gabriel Luong [:gl] (ΦωΦ)]

(In reply to Alexandra Lucinet, QA Mentor [:adalucinet] from comment #11)
> Reproduced with Firefox 33 beta 2 de build: the Add rule option via
> Inspector/Rules is not working.
> Verified as fixed on Mac OS X 10.9.4, a new rule is added with:
> - Firefox 33 beta 4 (Build ID: 20140915204924) de, ar, fr, it and ko builds
> - latest Aurora 34.0a2 (Build ID: 20140916004001) es-ES, da, gl and ja builds
> - latest Nightly 35.0a1 (Build ID: 20140916030204) hu, lt, nb-No and ru
> builds
> 
> During testing, I've encountered some issues:
> [1] On ko, gl and lt builds the Shortcut key for Inspector is not displayed
> (via Tools -> Web Developer); at a second attempt it appears and is the same
> as the one for Toggle Tools;
> [2] 'Add rule' is not translated on ar nor nb-NO builds;
>  
> Should I file a bug on this matter? Or those are known issues?

File please :)