Attachment pane view state machinery via keyboard shortcut (Alt+M) regressed
Categories
(Thunderbird :: Message Compose Window, defect)
Tracking
(Not tracked)
People
(Reporter: thomas8, Assigned: mkmelin)
References
(Regression)
Details
(Keywords: regression)
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
aleca
:
review+
|
Details | Diff | Splinter Review |
From Bug #1613004, comment 0:
Some other behaviour of the carefully-crafted shortcut-operated attachment pane state machinery is also broken (repeated Alt+M), e.g. with a focused but not selected attachment, unexpectedly bringing up the dialog whilst simultaneously hiding the bucket.
I'll file this as a stub, we can do more details as we go along.
Comment 1•5 years ago
|
||
Alice, can you please find this regression for us. Already broken in TB 68.6.
STR:
Start TB, write a message, press Alt+M: Result Empty attachment area appears. Then press Alt+M again.
Expected:
Empty attachment area is closed again.
Actual: File browser is opened :-(
Comment 2•5 years ago
|
||
Regression window:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=8e9201ae828118bbc3bceb4f7413fd67b38fe1ec&tochange=44e0b7bccc36e2fbea92430028eb2e57faca7099
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=219a897031a3fb57d5a263c1989043eb38e94165&tochange=c143aa387e91e365ef981af7a05aade8a7a6808a
Comment 3•5 years ago
|
||
Grrrr, what went broken in June last year, and the breakage made it into TB 68 beta at the time :-(
The offender must be somewhere below this line:
https://hg.mozilla.org/releases/comm-beta/rev/06327efa8b67bd2c793b8aedca893cdf45ccaf89#l3.1
Magnus, can you please get this fixed.
Alice, thank you so much, what would we do without you?
Comment 4•5 years ago
|
||
And maybe we should have a test for this. Pressing Alt+M twice on a new message and looking at the result isn't rocket science.
Reporter | ||
Comment 5•5 years ago
|
||
I have investigated this bug and found something interesting:
Pressing Alt+M with focus on empty attachmentBucket triggers both of the following events on <richlistbox is="attachment-list" id="attachmentBucket">
: onclick
and onkeypress
(in that order).
Assignee | ||
Comment 7•5 years ago
|
||
Remove the over-engineering here.
At the heart of the issue, we had the accesskey(!!) m of the attachmentBucketCount acting as double as a hotkey. In combination with controls on that label... But, this is just a label and I see no reason for it to have an accesskey which we use to go to editable places - which this is obviously not. Then how do one use an accesskey?With alt, so Alt+m. What was the hotkey? Alt-m => Wrong/double things getting triggered.
For the menuitem, there was a lot of code to do different things with focus, but I'm not convinced that was useful at all. (And working differently from the contacts pane, which do always get focus no matter how it was opened.) How that set a temporary attribute on an element to keep track of how we came there was just wrong.
Finally, the "checked" on the command should not be used at all anymore, it's part of the observes xul functionality that will be dropped at some point.
The autoCheck was just near by. Removed since it does nothing. The attribute is lower-case autocheck...
Assignee | ||
Comment 8•5 years ago
|
||
(Forgot to lint and such.)
Comment 9•5 years ago
|
||
Comment 10•5 years ago
|
||
Assignee | ||
Comment 11•5 years ago
|
||
We may have been too subtle with how to actually add files.
This adds button to make it clear.
Comment 12•5 years ago
|
||
Assignee | ||
Comment 13•5 years ago
|
||
Unbitrotted.
Comment 14•5 years ago
|
||
Comment 15•5 years ago
|
||
Don't forget to fix this in TB 68.x where it's sadly broken as well :-( - See comment #3. Thanks for the test :-)
Comment 16•5 years ago
|
||
(In reply to Magnus Melin [:mkmelin] from comment #7)
Remove the over-engineering here.
At the heart of the issue, we had the accesskey(!!) m of the attachmentBucketCount acting as double as a hotkey. In combination with controls on that label... But, this is just a label and I see no reason for it to have an accesskey which we use to go to editable places - which this is obviously not. Then how do one use an accesskey? With alt, so Alt+m. What was the hotkey? Alt-m => Wrong/double things getting triggered.
Well, before you remove the alleged "over-engineering", how about you try it in a working version?
EDIT: https://archive.mozilla.org/pub/thunderbird/nightly/2018/11/2018-11-21-10-00-42-comm-central/
Sure, the attachment bucket is not editable as such, but getting there conveniently was a major part of the original solution. Once there, you can continue using the keyboard for things like using the context menu to do other keyboard based actions, or hitting enter to open the file selection dialogue. What you call "over-engineering" was a finely tuned efficiency tool that two Thunderbird peers (myself and Aceman) worked on.
Comment 17•5 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/e83059b0ea0e
fix attachment pane toggling. r=aleca DONTBUILD
Assignee | ||
Comment 18•5 years ago
|
||
For 68: not exactly sure how to fix this properly there. "The minimal fix". Given we had no complaints about this, I'd tend to just let it ride the release trains.
Re comment 16: not sure what you're referring to, but with the patch things will still allow what you describe, just in less code and without the bug. With the added button how to add via keyboard will be discoverable, which (since we got no complaints) this bug shows the earlier situation was not.
Assignee | ||
Comment 19•5 years ago
|
||
UI improvements in bug 1625263.
Comment 20•5 years ago
|
||
Reporter | ||
Comment 21•5 years ago
|
||
(In reply to Jorg K (GMT+1) from comment #16)
...how about you try it in a working version?
EDIT: https://archive.mozilla.org/pub/thunderbird/nightly/2018/11/2018-11-21-10-00-42-comm-central/
Yes, the above version was still fully working as designed.
Fwiw, and ftr of history:
Bisecting on comm-central, 2018-03-12 - 2019-07-08:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=59c0f6e4825c4db83eb64c4dbd6ed11aee7be98e&tochange=2f0f868999d819e189bd67142aea132efb8a25c4
-
last working version where everything wrt attachment pane ux-efficiency was still working as originally designed (66.0a1):
https://archive.mozilla.org/pub/thunderbird/nightly/2018/12/2018-12-30-07-33-51-comm-central/ -
first version where things* started to break (66.0a1):
https://archive.mozilla.org/pub/thunderbird/nightly/2018/12/2018-12-31-09-44-43-comm-central/
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=dde486474a5eb462e8a3c7e935af15edd1e4c9ee&tochange=992cb1d8b84f15a340dc72c3a7f61eb4da31e945
*: single left click on attachment pane whitespace to attach, on empty or full bucket
That's different from the regression range of this bug (comment 2) because I also checked when single click on attachment pane whitespace was still fully working (related bug 1613004 just fixed half of that); whitespace-attaching broke long before attach dialogs started popping up at the wrong time (this bug 1613284).
Updated•5 years ago
|
Comment 22•4 years ago
|
||
More easily found if placed in the compose component
Description
•