Closed
Bug 1487087
Opened 6 years ago
Closed 6 years ago
Port bug 1457218 to TB: Remove support for button[type="menu-button"]
Categories
(Thunderbird :: General, enhancement)
Thunderbird
General
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 63.0
People
(Reporter: Paenglab, Assigned: arshad)
References
Details
Attachments
(2 files)
800 bytes,
patch
|
jorgk-bmo
:
review+
|
Details | Diff | Splinter Review |
24.86 KB,
patch
|
jorgk-bmo
:
review+
|
Details | Diff | Splinter Review |
We use button[type="menu-button"] in our notification boxes. For example the "Remind me later" button on the attachment reminder notification in composer. This will break after it's removal.
Comment 1•6 years ago
|
||
There's also the "menu-button" binding in common/bindings/toolbar.xml that extends "menu-button-base" (which is deleted in that bug): https://searchfox.org/comm-central/rev/936022fa3ff720477a181fbf6f162048c617a6b9/common/bindings/toolbar.xml#604. Are any of the consumers relevant to TB: https://searchfox.org/comm-central/search?q=toolbarbutton.xml%23menu-button&path= ?
Reporter | ||
Comment 2•6 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #1) > There's also the "menu-button" binding in common/bindings/toolbar.xml that > extends "menu-button-base" (which is deleted in that bug): > https://searchfox.org/comm-central/rev/ > 936022fa3ff720477a181fbf6f162048c617a6b9/common/bindings/toolbar.xml#604. Then we probably need to copy this binding. Or arshad has a better solution. > Are any of the consumers relevant to TB: > https://searchfox.org/comm-central/search?q=toolbarbutton.xml%23menu- > button&path= ? This is Seamonkey and doesn't relate to TB. SM is badly busted on trunk.
Comment 3•6 years ago
|
||
(In reply to Richard Marti (:Paenglab) from comment #2) > (In reply to Brian Grinstead [:bgrins] from comment #1) > > There's also the "menu-button" binding in common/bindings/toolbar.xml that > > extends "menu-button-base" (which is deleted in that bug): > > https://searchfox.org/comm-central/rev/ > > 936022fa3ff720477a181fbf6f162048c617a6b9/common/bindings/toolbar.xml#604. > > Then we probably need to copy this binding. Or arshad has a better solution. > > > Are any of the consumers relevant to TB: > > https://searchfox.org/comm-central/search?q=toolbarbutton.xml%23menu- > > button&path= ? > > This is Seamonkey and doesn't relate to TB. SM is badly busted on trunk. If that's the case then you shouldn't need to copy that binding for TB.
Comment 4•6 years ago
|
||
Bug 1457218 is now on mozilla-inbound and will be merged to mozilla-central in the next 4-10 hours. Arshad, how is the patch coming along?
Flags: needinfo?(arshdkhn1)
Assignee | ||
Comment 5•6 years ago
|
||
(In reply to Jorg K (GMT+2) from comment #4) > Bug 1457218 is now on mozilla-inbound and will be merged to mozilla-central > in the next 4-10 hours. > > Arshad, how is the patch coming along? Jorg, I am working on it.
Flags: needinfo?(arshdkhn1)
Assignee | ||
Comment 6•6 years ago
|
||
This patch works after applying patch[1] to moziila-central, which remove menu-button-base and replaces it with button-base. I still have to test menu-button in notification-bar, to see whether this is working there or not.
Attachment #9005201 -
Flags: review?(jorgk)
Assignee | ||
Comment 7•6 years ago
|
||
[1] - https://d3kxowhw4s8amj.cloudfront.net/file/data/5itc7b3ydsvs3ysy5bjg/PHID-FILE-o5lkka3j2br77vsyqqlg/D4529.diff
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → arshdkhn1
Comment 8•6 years ago
|
||
I'm sadly not an XBL expert. I've talked to Richard via IRC and he told me that there are two bindings affected: toolbarbutton, fixed in this patch, and "normal" button, used, for example, here: https://searchfox.org/comm-central/rev/e99c237cd3fd62b2a5ebd88e35194f8f0efbab11/mail/base/content/FilterListDialog.xul#92 https://searchfox.org/comm-central/rev/e99c237cd3fd62b2a5ebd88e35194f8f0efbab11/mail/components/addrbook/content/addressbook.xul#819 https://searchfox.org/comm-central/rev/e99c237cd3fd62b2a5ebd88e35194f8f0efbab11/mail/components/compose/content/MsgComposeCommands.js#2309 I have confirmed that with all three M-C, well, M-I patches and the patch here applied, the attachment notification is broken. Equally the "Get Map" and the "New" button in filters are broken, there are no menus any more. The "Get Map" button was going to show a selection of map providers and the filter "New" button also had a copy option. So what's the plan for those? Here's try run so you can see the damage: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=051b926daf75418a8f1093c36c4e668d8396ceea Given that we have about five hours left, how about copying those binding into C-C/common as we've done for all the other bindings?
Comment 9•6 years ago
|
||
Comment on attachment 9005201 [details] [diff] [review] menu-button-base.patch Thanks, this fixes the problem for toolbars.
Attachment #9005201 -
Flags: review?(jorgk) → review+
Reporter | ||
Comment 10•6 years ago
|
||
(In reply to Jorg K (GMT+2) from comment #8) > > Given that we have about five hours left, how about copying those binding > into C-C/common as we've done for all the other bindings? It already landed on m-c.
Reporter | ||
Comment 11•6 years ago
|
||
This fixes the button[type="menu-button"]. I copied the removed bindings. Only the removed parts and not the whole files. The buttons works again but the accessibility isn't restored.
Attachment #9005432 -
Flags: review?(jorgk)
Comment 12•6 years ago
|
||
Comment on attachment 9005432 [details] [diff] [review] button-menu-button.patch This works for me. "Get Map", "New" filter and attachment reminder work again. Mozmill test composition/test-attachment-reminder.js passes again. It's sad that we have to accumulate more technical debt by copying more and more bindings and losing accessibility functionality when doing so :-( But it's bustage time, so no choice. Arshad, please file a follow up.
Attachment #9005432 -
Flags: review?(jorgk) → review+
Comment 13•6 years ago
|
||
Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/60c67f7f2dc7 Use button-base in place of menu-button-base for toolbar buttons. r=jorgk https://hg.mozilla.org/comm-central/rev/d6a1a0b0d3aa Restore the button[type="menu-button"] bindings which were removed in bug 1457218. r=jorgk DONTBUILD
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Target Milestone: --- → Thunderbird 63.0
Version: unspecified → Trunk
You need to log in
before you can comment on or make changes to this bug.
Description
•