Closed Bug 158362 Opened 22 years ago Closed 19 years ago

spacebar does not scroll in print preview

Categories

(Core :: Print Preview, defect, P3)

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: joshgold, Assigned: MatsPalmgren_bugz)

References

()

Details

Attachments

(1 file, 4 obsolete files)

buildID: 2002061401

Hitting space in print preview mode doesn't scroll.  Only PageDown works.

(always reproducible)
1. Print preview
2. Press spacebar
this was also mentioned in bug 111432 comment 1
Blocks: 103890
Status: UNCONFIRMED → NEW
Ever confirmed: true
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → Future
*** Bug 285061 has been marked as a duplicate of this bug. ***
OS: Solaris → All
Hardware: Sun → All
Attached patch patch (obsolete) — Splinter Review
This makes spacebar scrolling in print preview work for me.
Attachment #188066 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 188066 [details] [diff] [review]
patch

>-                                  nsIDOMKeyEvent::DOM_VK_TAB, 0};
>+                                  nsIDOMKeyEvent::DOM_VK_TAB, nsIDOMKeyEvent::DOM_VK_SPACE, 0};
Don't need to list space here, it's always a char code.

>     keyEvent->GetShiftKey(&b);
>     if (b) return PR_FALSE;
> 
>     PRUint32 keyCode;
>     keyEvent->GetKeyCode(&keyCode);
>+    PRUint32 charCode;
>+    keyEvent->GetCharCode(&charCode);
>     PRInt32 i = 0;
>     while (kOKKeyCodes[i] != 0) {
>-      if (keyCode == kOKKeyCodes[i]) {
>+      if ((keyCode == kOKKeyCodes[i]) || (charCode == 0x20)) {
No point checking the charCode inside the loop. In fact, as Shift+Space is
legal for page up you might want to move it to before the shift test. Also you
should compare it to ' ' for readability.
Attachment #188066 - Flags: review?(neil.parkwaycc.co.uk) → review-
Attached patch patch2 (obsolete) — Splinter Review
Thanks, Neil. All your comments seems logical to me, but for some reason I need
to keep nsIDOMKeyEvent::DOM_VK_SPACE in, otherwise it doesn't work.
Also, I can't get Shift->spacebar scrolling to work.
Attachment #188066 - Attachment is obsolete: true
Neil, maybe you know why I get these results in comment 5? Thanks.
(In reply to comment #6)
>Neil, maybe you know why I get these results in comment 5? Thanks.
Ah, this is due to aaron's keydown prevents keypress code, so we need to allow
space twice, once as a keydown of DOM_VK_SPACE and once as a keypress of " ".
We also need to allow the shift key in both cases. Maybe it would be easier to
write some special-case code rather than trying to hook into the existing loop?
Or remove the keypress eventlistener, which should be safe, because the keydown
eventlisteners prevents keypress events of firing, not? But that doesn't seem to
change anything.
Attached patch patch3 (obsolete) — Splinter Review
Off course! The shift key also gets blocked. Why not remove that block? That
can't hurt anything, can it? And those keypress event listeners could also be
removed safely, I guess, since keydown also blocks keypress automatically.
Attachment #188092 - Attachment is obsolete: true
What I don't want to happen is for a keypress to either a) activate a shortcut
or b) run a script, when it shouldn't (see bug 245024 for a click example).
Comment on attachment 188327 [details] [diff] [review]
patch3

>-    mEventTarget->AddEventListener(NS_LITERAL_STRING("keypress"), this, true);
Although it's true that calling PreventDefault on the keydown now calls
PreventDefault on the keypress that doesn't handle the StopPropagation case.
r=me on the rest of the patch.
Attached patch patch4 (obsolete) — Splinter Review
Ok, I've put it back in this patch.
Attachment #188327 - Attachment is obsolete: true
Attachment #188594 - Flags: superreview?(jst)
Attached patch Patch rev. 5Splinter Review
Propagate DOM_VK_SPACE and ' ' with and without SHIFT (but nothing else).
I have tested this on Windows and Linux.
Assignee: rods → mats.palmgren
Attachment #188594 - Attachment is obsolete: true
Attachment #190212 - Flags: superreview?(bzbarsky)
Attachment #190212 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #188594 - Flags: superreview?(jst)
Attachment #190212 - Flags: superreview?(bzbarsky) → superreview+
Attachment #190212 - Flags: review?(neil.parkwaycc.co.uk) → review+
Attachment #190212 - Flags: approval1.8b4?
Attachment #190212 - Flags: approval1.8b4? → approval1.8b4+
Checked in to trunk 2005-07-25 17:21 PDT

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Target Milestone: Future → ---
Verified FIXED using build 2005-07-26-05 on Windows XP SeaMonkey trunk.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: