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
:
: Markus Stange [:mstange]
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 User image 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 User image 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 User image 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 User image 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 User image 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 User image 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 User image Smokey Ardisson (offline for a while; not following bugs - do not email) 2008-01-02 20:14:12 PST
--> Markus
Comment 7 User image 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 User image 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 User image 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 User image 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 User image 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 User image philippe (part-time) 2008-01-05 16:47:22 PST
Thank you !
Comment 13 User image 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 User image 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 User image 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 User image 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 User image 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.