Last Comment Bug 326035 - Page Up and Page Down do not hide cursor
: Page Up and Page Down do not hide cursor
Status: RESOLVED FIXED
: verified1.8.1.12
Product: Core
Classification: Components
Component: Widget: Cocoa (show other bugs)
: unspecified
: PowerPC Mac OS X
: -- trivial with 1 vote (vote)
: mozilla1.9beta3
Assigned To: Markus Amalthea Magnuson
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-02-05 20:53 PST by Chris Lawson (gone)
Modified: 2008-01-30 20:57 PST (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Hide cursor when scrolling with Page Up/Down (1.08 KB, patch)
2008-01-02 08:23 PST, Markus Amalthea Magnuson
jaas: review+
roc: superreview+
dveditz: approval1.8.1.12+
mtschrep: approval1.9+
Details | Diff | Splinter Review

Description Chris Lawson (gone) 2006-02-05 20:53:40 PST
Standard Mac OS behaviour is to hide the cursor when scrolling through a page with the keyboard. We do the Right Thing(tm) for the arrow keys, Home, and End, but for some reason, we don't hide the cursor when scrolling with Page Up or Page Down.
Comment 1 Smokey Ardisson (offline for a while; not following bugs - do not email) 2007-11-14 11:56:39 PST
Mass-reassign of bugs still assigned to pinkerton to nobody; filter on "NoMoPinkBugsInCamino".
Comment 2 Markus Amalthea Magnuson 2008-01-02 08:23:43 PST
Created attachment 295099 [details] [diff] [review]
Hide cursor when scrolling with Page Up/Down

I've been annoyed by this for quite some time, so here's a fix. Not sure who to ask for a review, so starting with Chris who reported this. Feel free to point me to someone else if appropriate.

Also, I am not familiar with the widget code; obviously someone explicitly put in those checks for page up/down keys, and I can't figure out why. Hopefully because hiding the cursor on page scroll wasn't standard behaviour at that time, and someone just forgot to change it later on.
Comment 3 Chris Lawson (gone) 2008-01-02 08:39:59 PST
Comment on attachment 295099 [details] [diff] [review]
Hide cursor when scrolling with Page Up/Down

Josh would be a better guy to ask for review in Cocoa widget code. cbarrett might also have something useful to say. I'm not even sure I'm eligible to review Cocoa widget code, but regardless, I definitely don't feel comfortable doing so since it can potentially affect a lot more than just Camino now that Firefox is using it too.
Comment 4 Chris Lawson (gone) 2008-01-02 08:42:18 PST
And since this appears to be a Cocoa widget bug, not strictly a Camino bug, I'm kicking it to Core.
Comment 5 Håkan Waara 2008-01-02 08:46:32 PST
CVS blame for that line doesn't mention anything interesting. It was last touched by josh for the mega-cleanup patch in bug 383560. AFAICS this change makes sense even in the XUL world.
Comment 6 Smokey Ardisson (offline for a while; not following bugs - do not email) 2008-01-02 20:14:12 PST
--> Markus
Comment 7 Robert O'Callahan (:roc) (email my personal email if necessary) 2008-01-05 01:30:03 PST
Comment on attachment 295099 [details] [diff] [review]
Hide cursor when scrolling with Page Up/Down

It's been there since the beginning of time.
Comment 8 Samuel Sidler (old account; do not CC) 2008-01-05 01:55:45 PST
Comment on attachment 295099 [details] [diff] [review]
Hide cursor when scrolling with Page Up/Down

(In reply to comment #7)
> It's been there since the beginning of time.

Yeah... hyatt added this in revision 1.16 of this file (line 2303), with a bunch of other code and the description "Implement key events in cocoa. Rough, but works. Not part of build."

I guess it worked for this long...

Camino would like to get this fix on the branch as well. It's simple and straightforward and NPOTB on branch since Firefox doesn't use widget/cocoa.
Comment 9 Samuel Sidler (old account; do not CC) 2008-01-05 01:56:28 PST
Comment on attachment 295099 [details] [diff] [review]
Hide cursor when scrolling with Page Up/Down

Oh, right. I guess I should request trunk approval too...
Comment 10 Smokey Ardisson (offline for a while; not following bugs - do not email) 2008-01-05 12:42:29 PST
I'll check this in for Markus once the Firefox tree is no longer red.
Comment 11 Smokey Ardisson (offline for a while; not following bugs - do not email) 2008-01-05 15:35:45 PST
Checking in widget/src/cocoa/nsChildView.mm;
/cvsroot/mozilla/widget/src/cocoa/nsChildView.mm,v  <--  nsChildView.mm
new revision: 1.291; previous revision: 1.290
done
Comment 12 philippe (part-time) 2008-01-05 16:47:22 PST
Thank you !
Comment 13 Daniel Veditz [:dveditz] 2008-01-10 16:11:53 PST
Comment on attachment 295099 [details] [diff] [review]
Hide cursor when scrolling with Page Up/Down

approved for 1.8.1.12, a=dveditz for release-drivers
Comment 14 Smokey Ardisson (offline for a while; not following bugs - do not email) 2008-01-10 19:39:45 PST
Checked in on the MOZILLA_1_8_BRANCH for Markus:

Checking in widget/src/cocoa/nsChildView.mm;
/cvsroot/mozilla/widget/src/cocoa/nsChildView.mm,v  <--  nsChildView.mm
new revision: 1.120.2.18; previous revision: 1.120.2.17
done

Thanks, Markus!
Comment 15 juan becerra [:juanb] 2008-01-30 17:32:49 PST
This works in the latest trunk, but it does not work in: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; fr; rv:1.8.1.12) Gecko/2008012822 Firefox/2.0.0.12

Cursor still shows when doing Page Up and Page Down. Removing the "fixed1.8.1.12" keyword.
Comment 16 Smokey Ardisson (offline for a while; not following bugs - do not email) 2008-01-30 19:56:36 PST
Juan, this is a Cocoa widget bug, so on the 1_8 branch you must use a Camino build to verify this, not a Firefox (Widget:Mac) build.

Restoring the fixed1.8.1.12 keyword.
Comment 17 juan becerra [:juanb] 2008-01-30 20:57:12 PST
This works in Camino Version 1.6b3pre (1.8.1.12pre 2008013001). Changed keyword to "verified1.8.1.12"

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