655501 - Ctrl+Shift+Q doesn't work when a sidebar is open

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Anas Husseini [:linostar]]

Steps to reproduce:

1- Snap right or left to show a sidebar.
2- Press Ctrl+Shift+Q.

Expected result: Portrait/Landscape mode is toggled
Actual result: View mode isn't toggled, and newtab button disappears.

Comment 1

[Anas Husseini [:linostar]]

Attachment: patch

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 536081 [details] [diff] [review]
patch

I don't think this works. If you are here it means the shortcuts has been caught by the chrome process already, you need to look why the content side take this shortcut and stop it instead.

Comment 3

[Anas Husseini [:linostar]]

I tested the patch and it worked, with no side effects apparently. When one of the sidebars is open, what seems catching the keypress event is the viewport... or something else.

Comment 4

[Anas Husseini [:linostar]]

Attachment: patch v2

Comment 5

[Anas Husseini [:linostar]]

Attachment: patch v2

Comment 6

[Daniel Holbert [:dholbert]]

Anas: just as a heads-up, the patch will be much easier for someone to check in correctly if you post an updated version with the author & commit-message metadata included at the top of the patch.

For details on how to do that, see:
https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3f

If you don't get around to that for this bug, then you could simply post a comment here with a suggested checkin message and the author name/email you'd like to use in mozilla-central (if it's different from your bugmail address), and the person checking your patch in can add those at checkin-time.

Comment 7

[Anas Husseini [:linostar]]

Attachment: same patch with username and commit message

Comment 8

[Anas Husseini [:linostar]]

(In reply to comment #6)
> Anas: just as a heads-up, the patch will be much easier for someone to check
> in correctly if you post an updated version with the author & commit-message
> metadata included at the top of the patch.
> 

I adjusted the patch so that it includes the metadata you mentioned.

Comment 9

[Daniel Holbert [:dholbert]]

(In reply to comment #8)
> I adjusted the patch so that it includes the metadata you mentioned.

Thanks!  For future reference, the commit message should also include the reviewer at the end (e.g. "r=vingtetun").  I added that before pushing, in this case.

Patch landed on mozilla-inbound, & it'll be merged to mozilla-central soon.
http://hg.mozilla.org/integration/mozilla-inbound/rev/efb81f7ce02a

Comment 10

[Mounir Lamouri (:mounir)]

Merged:
http://hg.mozilla.org/mozilla-central/rev/efb81f7ce02a

Comment 11

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Verified: 
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv8.0a1) Gecko/20110711 Firefox/8.0a1 Fennec/8.0a1
Device: MacBook Pro
OS: Mac OS X 10.6.7

Mozilla/5.0 (X11; Linux i686; rv8.0a1) Gecko/20110711 Firefox/8.0a1 Fennec/8.0a1
Device: MacBook Pro / VMWare
OS: Linux ubuntu 2.6.32-24-generic

Mozilla/5.0 (Windows NT 5.1; rv8.0a1) Gecko/20110711 Firefox/8.0a1 Fennec/8.0a1
Device: MacBook Pro / VMWare
OS: Windows XP