Closed Bug 940843 Opened 7 years ago Closed 7 years ago

Print preview Alt+C doesn't activate "close" button anymore (& similar for other print preview UI with alt-keys)

Categories

(Core :: Print Preview, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla28
Tracking Status
firefox25 --- affected
firefox26 --- affected
firefox27 --- affected
firefox28 --- verified

People

(Reporter: dholbert, Assigned: masayuki)

References

Details

(Keywords: regression, Whiteboard: [good first verify] [bugday-20140108])

Attachments

(1 file)

STR:
 1. View about:blank
 2. File | Print Preview
 3. Alt+c to activate "Close" button. (or Alt+p, Alt+u, etc, to activate other UI)

ACTUAL RESULTS: Nothing happens.
EXPECTED RESULT: Close button (or whatever you chose) should be activated.

From a local mozregression run, it appears this regressed a few months ago:
{
Last good nightly: 2013-07-25
First bad nightly: 2013-07-26

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a4c1961bf723&tochange=46d73e889cb4
}

There isn't anything print-related in that pushlog, but Masayuki landed a bunch of keypress handling stuff for bug 501496 that day -- that seems like the most likely related thing, at first glance.
(This isn't about:blank specific, of course; I just chose that as a trivial testcase)
I can reproduce this bug on Win8.1 too. I guess that print preview consumes keydown event before ESM handles accesskey if bug 501496 is the actual cause.
OS: Linux → All
Hardware: x86_64 → All
Access key is handled by ESM::PreHandleEvent() with keypress event without checking defaultPrevented value.

Therefore, before bug 501496, even if keydown event is consumed by content, following keypress event worked as modifier key. This means that such key event may have caused "double action".

Anyway, we need to kill only the event propagation of keydown events of printable keys. But looks like nsPrintPreviewListener approach looks hacky to me. I believe that it should be killed by PresShell level but such change is really risky and too big change.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Attached patch PatchSplinter Review
nsPrintPreviewListener shouldn't consume keydown event because it kills following keypress event. Instead, it should just stop propagation of keydown event both in default event group and system event group.

https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=4101d7df065a
Attachment #8336717 - Flags: review?(matspal)
Attachment #8336717 - Flags: review?(bugs)
Comment on attachment 8336717 [details] [diff] [review]
Patch

The code changes looks fine to me, so r=mats if Olli thinks this is the
right fix from an event handling point of view.
Attachment #8336717 - Flags: review?(matspal) → review+
Attachment #8336717 - Flags: review?(bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/f73da8ab5911
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Flags: in-testsuite?
Flags: in-qa-testsuite?
Whiteboard: [good first verify]
Check on Firefox Nightly 29. The fix works for me. 

[bugday-20120108]
[bugday-20140108]
(In reply to AShickur Rahman from comment #9)
> Check on Firefox Nightly 29. The fix works for me. 

Would you mind checking the latest Aurora build? (http://aurora.mozilla.org)
Checked on Aurora build 28.0a2 (2014-01-08), linux x86_64. Fix works for me.
Whiteboard: [good first verify] → [good first verify] [bugday-20140108]
Works fine in latest aurora 28.0a2 (2014-01-21), Win7 x32
work fine in Nightly build 29.0a1 (2014-01-21), Win 7 x64
[bugday-20140120]
[bugday-20140122] - works fine in Latest aurora and Nightly build - Win 7 x64
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.