The default bug view has changed. See this FAQ.

Can't close Style Editor with cmd+w

VERIFIED FIXED in Firefox 11

Status

()

Firefox
Developer Tools: Style Editor
P2
normal
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: jdm, Assigned: cedricv)

Tracking

({verified-beta})

unspecified
Firefox 12
x86
Mac OS X
verified-beta
Points:
---

Firefox Tracking Flags

(firefox11+ verified, firefox12 verified)

Details

(Whiteboard: [styleeditor][qa!])

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
The Style Editor acts like a separate window, similar to the Download Manager, but I cannot close it with the regular window-closing keyboard shortcut.

Updated

5 years ago
Priority: -- → P2
(Assignee)

Updated

5 years ago
Assignee: nobody → cedricv
(Assignee)

Updated

5 years ago
Whiteboard: [styleeditor]
(Assignee)

Comment 1

5 years ago
Created attachment 591398 [details] [diff] [review]
patch v1
Attachment #591398 - Flags: review?(paul)

Updated

5 years ago
Attachment #591398 - Flags: review?(paul) → review+

Updated

5 years ago
Whiteboard: [styleeditor] → [styleeditor][land-in-fx-team]
(Assignee)

Comment 2

5 years ago
We should nominate this for aurora approval after bake time in m-c.

Updated

5 years ago
tracking-firefox11: --- → ?

Comment 3

5 years ago
Created attachment 591427 [details] [diff] [review]
patch v1 - rebased

Comment 4

5 years ago
https://hg.mozilla.org/integration/fx-team/rev/7b66794634ec
status-firefox12: --- → fixed
Whiteboard: [styleeditor][land-in-fx-team] → [styleeditor][fixed-in-fx-team]

Comment 5

5 years ago
I will a? this patch once in central.
Whiteboard: [styleeditor][fixed-in-fx-team] → [styleeditor][fixed-in-fx-team][addToFirefox11]
Duplicate of this bug: 715304
https://hg.mozilla.org/mozilla-central/rev/7b66794634ec
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [styleeditor][fixed-in-fx-team][addToFirefox11] → [styleeditor][addToFirefox11]
Target Milestone: --- → Firefox 12
Comment on attachment 591427 [details] [diff] [review]
patch v1 - rebased

[Approval Request Comment]
Regression caused by (bug #): Not a regression, new feature.
User impact if declined: User won't be able to close Style Editor window with common shortcut.
Testing completed (on m-c, etc.):  on m-c, local testing.
Risk to taking this patch (and alternatives if risky): Not risky. Defines simple xul keyboard shortcut for closing the window.
Attachment #591427 - Flags: approval-mozilla-aurora?

Comment 9

5 years ago
Comment on attachment 591427 [details] [diff] [review]
patch v1 - rebased

[Triage Comment]
Regression in new feature, approved for Aurora.
Attachment #591427 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/a04e7209a49c
status-firefox11: --- → fixed

Updated

5 years ago
Whiteboard: [styleeditor][addToFirefox11] → [styleeditor]
Aurora is supposed to be string frozen...

Comment 12

5 years ago
Alex, you approved a string-change bug here for aurora.

Can we get this backed out?
Didn't catch the fact that there was a string change here - only saw the shortcut addition. Yes, we can back this out, or possibly remove the string if the devtools guys are comfortable with that change (not sure where the string shows up in the UI).

Rob - can you take next action on this?
tracking-firefox11: ? → +
(In reply to Alex Keybl [:akeybl] from comment #13)
> Didn't catch the fact that there was a string change here - only saw the
> shortcut addition. Yes, we can back this out, or possibly remove the string
> if the devtools guys are comfortable with that change (not sure where the
> string shows up in the UI).
> 
> Rob - can you take next action on this?

So we're ok with not having a keyboard shortcut on beta (after the merge)? I can back out if that's the call, but I'd really prefer to have this in.
(In reply to Rob Campbell [:rc] (robcee) from comment #14)
> So we're ok with not having a keyboard shortcut on beta (after the merge)? I
> can back out if that's the call, but I'd really prefer to have this in.

Our options are

1) Back out the change
2) Figure out a way to get this feature in without a string
3) Try to find another option for localizing the feature with Axel

If you'd like to pursue #3, please start an email thread with us to discuss further.

Comment 16

5 years ago
Given that this is a command key, l10n-merge could do the right thing.

I'm concerned about the amount of string-freeze bustages in this cycle, at large, though.

Comment 17

5 years ago
(In reply to Axel Hecht [:Pike] from comment #16)
> Given that this is a command key, l10n-merge could do the right thing.

Sorry, I don't know our l10n infrastructure as well as I should - is this a suggestion for #3 (another option for localizing)?  How much trouble is that, can we help make it easier?

Comment 18

5 years ago
l10n-merge is the jargon for our build-time fallback to en-US for missing strings in localizations. So with the status-quo, we're falling back to crtl-w if there's no localization, which is probably right anyway, as control keys should not be localized commonly.

That doesn't make things good, it just makes them OK to stay as is. Bugs like this should be found before landing on central, or on central. If it's not baked enough on central that it needs string fixes to not suck, switch it off.
Alright then. Based on this it sounds like we might be able to get away with it. It was kind of a stupid omission to not get that in before aurora, and now here we are at the end of the cycle... Balls were dropped then rolled along the floor.

Axel, let us know if this causes any problems or if you need any other action. Would landing an extra l10n comment to the file be more helpful or worse at this point?
> Based on this it sounds like we might be able to get away with it.

https://l10n-stage-sj.mozilla.org/shipping/dashboard?tree=fx_aurora

IMO situation is not good thanks to the last two bugs that broke string freeze.

Honestly, right now I don't understand if I have to add the missing string or wait for this change to be backed out and my locale to turn back green. Please tell us what to do (i.e. what you plan to do), since we're just a couple of days from the end of this cycle!
(In reply to Francesco Lodolo [:flod] from comment #20)
> > Based on this it sounds like we might be able to get away with it.
> 
> https://l10n-stage-sj.mozilla.org/shipping/dashboard?tree=fx_aurora
> 
> IMO situation is not good thanks to the last two bugs that broke string
> freeze.
> 
> Honestly, right now I don't understand if I have to add the missing string
> or wait for this change to be backed out and my locale to turn back green.
> Please tell us what to do (i.e. what you plan to do), since we're just a
> couple of days from the end of this cycle!

I don't know enough about l10n to fully advise but let me put it this way - if it's normal to land a string at this point on Aurora and it doesn't carry risk, feel free. Otherwise what Axel's saying is that the keyboard shortcut will suffice without needing localizations.

Comment 22

5 years ago
I'll do a post in .l10n, and one in .planning.
Whiteboard: [styleeditor] → [styleeditor][qa+]
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0) Gecko/20100101 Firefox/11.0

Verified the change on the Firefox 11 beta 1: CMD(Windows button)+w closes the style editor window.
status-firefox11: fixed → verified
Keywords: verified-beta
Whiteboard: [styleeditor][qa+] → [styleeditor][qa!:11][qa+]
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:12.0) Gecko/20100101 Firefox/12.0
Verified the fix on latest Firefox beta (12.0b1)
Status: RESOLVED → VERIFIED
status-firefox12: fixed → verified
Whiteboard: [styleeditor][qa!:11][qa+] → [styleeditor][qa!]
You need to log in before you can comment on or make changes to this bug.