556752 - Add-on Builder generates add-on which eat all key events

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Developer Pages, defect, P3)

Description

[dynamis (Tomoya ASAI)]

Current Add-on Builder
https://addons.mozilla.org/en-US/developers/tools/builder
will generate ff-overlay.xul which includes:

  <keyset id="mainKeyset">
    <key id="key_openSidebar_{name}" command="viewSidebar_{name}" />
  </keyset>

This key element eat all key events and beginner developers cannot notice about it, cannot find how to fix it. 

Instead, generate code like this:

  <!-- set key attribute and change modifiers to enable accesskey
    -- see: https://developer.mozilla.org/en/XUL/key -->
  <keyset id="mainKeyset">
    <key key="" modifiers="accel" id="key_openSidebar_{name}" command="viewSidebar_{name}" />
  </keyset>

Comment 1

[Justin Scott [:fligtar]]

Cesar, Jorge, thoughts?

Comment 2

[Jorge Villalobos [:jorgev] (he/him)]

Looks like the bug report is correct. Furthermore, I don't think the Add-on Builder should generate shortcut keys, given how difficult it is to get them right and not conflict with anything else.

Comment 3

[Justin Scott [:fligtar]]

Jorge, would you be willing to write a patch for fizzypop to fix this? We still have a good amount of add-ons built using the packager.

http://svn.mozilla.org/projects/fizzypop/trunk/

Comment 4

[Jorge Villalobos [:jorgev] (he/him)]

Attachment: Patch V1. Axe all the shortcut key code from the overlay template

I need to know who reviews this code.

This patch removes the shortcut key code entirely. It looks like fixing it would take a lot of work, and either way we shouldn't be encouraging the use of shortcut keys.

Comment 5

[Justin Scott [:fligtar]]

mfinkle and cesar are responsible for fizzypop I think.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 486462 [details] [diff] [review]
Patch V1. Axe all the shortcut key code from the overlay template

Looks OK to me. We don't seem to use <key> anywhere else in the extension templates.

Comment 7

[Jorge Villalobos [:jorgev] (he/him)]

Fixed, r76510.

Comment 8

[Jorge Villalobos [:jorgev] (he/him)]

Turns out that I didn't commit to the right repo :/ (bug 556754 comment #12)
Reopening to get this right.

Comment 9

[Jorge Villalobos [:jorgev] (he/him)]

Committing again on r82041.

Comment 10

[Jorge Villalobos [:jorgev] (he/him)]

Verified fixed.