Closed Bug 1061003 Opened 6 years ago Closed 5 years ago

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

Categories

(DevTools :: Inspector, defect)

x86
macOS
defect
Not set

Tracking

(firefox32 unaffected, firefox33 verified, firefox34 verified, firefox35 verified)

VERIFIED FIXED
Firefox 35
Tracking Status
firefox32 --- unaffected
firefox33 --- verified
firefox34 --- verified
firefox35 --- verified

People

(Reporter: dcamp, Assigned: gl)

Details

Attachments

(1 file)

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.
Assignee: nobody → gabriel.luong
Attachment #8482875 - Flags: review?(fayearthur) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/fae611216e7f
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 35
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
Attachment #8482875 - Flags: approval-mozilla-aurora?
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+
Attachment #8482875 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: needinfo?(gabriel.luong)
(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.
Flags: needinfo?(gabriel.luong)
I see. I misunderstood your comment as this fix was already on beta. Please request beta approval and let's get this uplifted.
Flags: needinfo?(gabriel.luong)
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
Attachment #8482875 - Flags: approval-mozilla-beta?
Flags: needinfo?(gabriel.luong)
Comment on attachment 8482875 [details] [diff] [review]
1061003.patch

Beta+
Attachment #8482875 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
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?
Status: RESOLVED → VERIFIED
Flags: needinfo?(gabriel.luong)
(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 :)
Flags: needinfo?(gabriel.luong)
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.