Closed Bug 1561650 Opened 5 years ago Closed 5 years ago

alt-home does not show Thunderbird start page

Categories

(Thunderbird :: Mail Window Front End, defect)

Product:
Thunderbird
Component:
Mail Window Front End
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(thunderbird_esr68+ fixed)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 70.0
Tracking Flags:
Tracking Status
thunderbird_esr68 + fixed

People

(Reporter: wsmwk, Assigned: darktrojan)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

Bug 1561650 - Ignore unused modifier keys on keydown events in <tree>
47 bytes, text/x-phabricator-request
jorgk-bmo
: approval-comm-esr68+
Details | Review

regression - alt+home isn't showing the start page, it acts like a bare home key

Seen on beta 68 and nightly. Not sure if it regressed earlier

Alice, can you please find the regression for us.

tracking-thunderbird68: --- → +
Flags: needinfo?(alice0775)

What is the start page? I do not remember it.

Flags: needinfo?(alice0775)

You get the configured start page from "Options > General, Thunderbird Start Page" displayed in the message preview. Try it in TB 60.

(In reply to Jorg K (GMT+2) from comment #3)

You get the configured start page from "Options > General, Thunderbird Start
Page" displayed in the message preview. Try it in TB 60.

Okay, I will

Thanks, Alice.

Brian, do you have any idea why that tree conversion would have affected what Alt+Home triggers?

Flags: needinfo?(bgrinstead)

These keydown handlers wouldn't run if the Alt key was pressed (no "alt" in the modifiers attribute), but when converted to CE they do run.

(In reply to Geoff Lankow (:darktrojan) from comment #7)

These keydown handlers wouldn't run if the Alt key was pressed (no "alt" in the modifiers attribute), but when converted to CE they do run.

Good catch. If this is the case (missing modifiers attribute on XBL handler causing the handler to not run), then I think this would be a behavior change with many XBL -> CE conversions, where we are just checking if the key matches and checking that modifiers that are explicitly specified do match.

A few thoughts:

  • This could be fixed as a one-off in places where we actually rely on this behavior, or we could go back and change previous conversions to match the behavior. It depends on how much consumers are intending for / relying on this behavior. I lean towards the former, at least to figure out what approach works best (using this regression as a test case). Then we might want to go and backfill.
  • If the "no modifier specified, so don't run handler" is the most common place this comes up, we could have a helper function (in a new JSM?) like hasNoModifiers(event) which returns basically !["Alt", "Control", "Meta", "Shift"].some(modifier => aEvent.getModifierState(modifier)); and gets used in the same place as the event.key / event.keyCode checks.
  • There's some semi-relevant code called eventMatchesKey that does some modifier checks for XUL <key> elements: https://searchfox.org/mozilla-central/rev/8a990595ce6d5ed07ace2d4d5d86cc69aec90bde/browser/base/content/utilityOverlay.js#732. I guess we could make a version of that like eventMatchesModifiers which passes in the expected modifiers as a JS object instead of as an attribute.
Flags: needinfo?(bgrinstead)

Geoff, weren't you going to file an M-C bug pointing out the things which might go wrong there?

Flags: needinfo?(geoff)
Regressed by: deforestation
See Also: → 1566663
Flags: needinfo?(geoff)
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/integration/autoland/rev/59456fe10bea
Ignore unused modifier keys on keydown events in <tree> r=vporof

Comment on attachment 9078555 [details]
Bug 1561650 - Ignore unused modifier keys on keydown events in <tree>

Since somehow the bug is on the TB product, I can apply our flags here. I'll put it only our release branch, in fact, I could have put it onto the beta branch, had it been available yesterday.

Attachment #9078555 - Flags: approval-comm-esr68+
Assignee: nobody → geoff
Status: NEW → ASSIGNED
Target Milestone: --- → Thunderbird 70.0
Backout by ccoroiu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a1e3fabe0ca9
Backed out changeset 59456fe10bea for chrome failures at toolkit/content/tests/chrome/test_tree.xul on a CLOSED TREE

Backed out changeset 59456fe10bea (Bug 1561650) for chrome failures at toolkit/content/tests/chrome/test_tree.xul.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=59456fe10bea649b2b263330ba11ca28bea8a3e2

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=256891570&repo=autoland&lineNumber=22700

Backout link: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=a1e3fabe0ca935f68f7a9f73dc2a9e0daee9aaca

[task 2019-07-17T10:20:17.260Z] 10:20:17     INFO - TEST-PASS | toolkit/content/tests/chrome/test_tree.xul | tree selection UI key down with accel count 
[task 2019-07-17T10:20:17.261Z] 10:20:17     INFO - Buffered messages finished
[task 2019-07-17T10:20:17.263Z] 10:20:17     INFO - TEST-UNEXPECTED-FAIL | toolkit/content/tests/chrome/test_tree.xul | tree selection UI key down with accel currentIndex - got 1, expected 2
[task 2019-07-17T10:20:17.264Z] 10:20:17     INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:320:16
[task 2019-07-17T10:20:17.264Z] 10:20:17     INFO - testtag_tree_TreeSelection_State@chrome://mochitests/content/chrome/toolkit/content/tests/chrome/tree_shared.js:1567:3
[task 2019-07-17T10:20:17.264Z] 10:20:17     INFO - testtag_tree_TreeSelection_UI@chrome://mochitests/content/chrome/toolkit/content/tests/chrome/tree_shared.js:637:3
[task 2019-07-17T10:20:17.266Z] 10:20:17     INFO - testtag_tree@chrome://mochitests/content/chrome/toolkit/content/tests/chrome/tree_shared.js:66:3
[task 2019-07-17T10:20:17.267Z] 10:20:17     INFO - async*SimpleTest_setTimeoutShim@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:684:43
[task 2019-07-17T10:20:17.267Z] 10:20:17     INFO - onload@chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_tree.xul:1:1
[task 2019-07-17T10:20:17.268Z] 10:20:17     INFO - rval@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:146:30
[task 2019-07-17T10:20:17.269Z] 10:20:17     INFO - Not taking screenshot here: see the one that was previously logged
[task 2019-07-17T10:20:17.271Z] 10:20:17     INFO - TEST-UNEXPECTED-FAIL | toolkit/content/tests/chrome/test_tree.xul | tree selection UI key down with accel TreeSelection currentIndex - got 1, expected 2
[task 2019-07-17T10:20:17.272Z] 10:20:17     INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:320:16
[task 2019-07-17T10:20:17.273Z] 10:20:17     INFO - testtag_tree_TreeSelection_State@chrome://mochitests/content/chrome/toolkit/content/tests/chrome/tree_shared.js:1568:3
[task 2019-07-17T10:20:17.274Z] 10:20:17     INFO - testtag_tree_TreeSelection_UI@chrome://mochitests/content/chrome/toolkit/content/tests/chrome/tree_shared.js:637:3
[task 2019-07-17T10:20:17.275Z] 10:20:17     INFO - testtag_tree@chrome://mochitests/content/chrome/toolkit/content/tests/chrome/tree_shared.js:66:3
[task 2019-07-17T10:20:17.284Z] 10:20:17     INFO - async*SimpleTest_setTimeoutShim@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:684:43
[task 2019-07-17T10:20:17.285Z] 10:20:17     INFO - onload@chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_tree.xul:1:1
[task 2019-07-17T10:20:17.286Z] 10:20:17     INFO - rval@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:146:30
[task 2019-07-17T10:20:17.287Z] 10:20:17     INFO - TEST-PASS | toolkit/content/tests/chrome/test_tree.xul | tree selection UI key down with accel selection [1] 
[task 2019-07-17T10:20:17.288Z] 10:20:17     INFO - TEST-PASS | toolkit/content/tests/chrome/test_tree.xul | tree selection UI key down with accel range selection [1] 
[task 2019-07-17T10:20:17.289Z] 10:20:17     INFO - TEST-PASS | toolkit/content/tests/chrome/test_tree.xul | key up with accel select event not fired 
[task 2019-07-17T10:20:17.291Z] 10:20:17     INFO - TEST-PASS | toolkit/content/tests/chrome/test_tree.xul | tree selection UI key up with accel count 
[task 2019-07-17T10:20:17.292Z] 10:20:17     INFO - Not taking screenshot here: see the one that was previously logged
[task 2019-07-17T10:20:17.293Z] 10:20:17     INFO - TEST-UNEXPECTED-FAIL | toolkit/content/tests/chrome/test_tree.xul | tree selection UI key up with accel currentIndex - got 4, expected 3
[task 2019-07-17T10:20:17.294Z] 10:20:17     INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:320:16
[task 2019-07-17T10:20:17.295Z] 10:20:17     INFO - testtag_tree_TreeSelection_State@chrome://mochitests/content/chrome/toolkit/content/tests/chrome/tree_shared.js:1567:3
[task 2019-07-17T10:20:17.296Z] 10:20:17     INFO - testtag_tree_TreeSelection_UI@chrome://mochitests/content/chrome/toolkit/content/tests/chrome/tree_shared.js:656:3
[task 2019-07-17T10:20:17.297Z] 10:20:17     INFO - testtag_tree@chrome://mochitests/content/chrome/toolkit/content/tests/chrome/tree_shared.js:66:3
[task 2019-07-17T10:20:17.298Z] 10:20:17     INFO - async*SimpleTest_setTimeoutShim@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:684:43
[task 2019-07-17T10:20:17.298Z] 10:20:17     INFO - onload@chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_tree.xul:1:1
[task 2019-07-17T10:20:17.299Z] 10:20:17     INFO - rval@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:146:30
[task 2019-07-17T10:20:17.300Z] 10:20:17     INFO - Not taking screenshot here: see the one that was previously logged
[task 2019-07-17T10:20:17.301Z] 10:20:17     INFO - TEST-UNEXPECTED-FAIL | toolkit/content/tests/chrome/test_tree.xul | tree selection UI key up with accel TreeSelection currentIndex - got 4, expected 3
[task 2019-07-17T10:20:17.302Z] 10:20:17     INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:320:16
Flags: needinfo?(geoff)

Re-landing now with a change and a successful Try run.

Flags: needinfo?(geoff)
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/integration/autoland/rev/ba21fb943434
Ignore unused modifier keys on keydown events in <tree> r=vporof

https://hg.mozilla.org/mozilla-central/rev/ba21fb943434

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED

Trick to get this off my query: releases/comm-esr68/rev

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: