Last Comment Bug 163174 - [FIX] Support CSS3 cursors
: [FIX] Support CSS3 cursors
Status: RESOLVED FIXED
: css3, testcase
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: -- enhancement with 3 votes (vote)
: ---
Assigned To: Mats Palmgren (:mats)
: Hixie (not reading bugmail)
Mentors:
http://www.w3.org/TR/css3-ui/#cursor
Depends on: context-menu-cursor 275173 275174 38447 48839 258966 260713
Blocks: 280331
  Show dependency treegraph
 
Reported: 2002-08-16 19:17 PDT by Tim Powell
Modified: 2009-04-17 11:15 PDT (History)
25 users (show)
asa: blocking1.8a3-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Implement remaining CSS3 cursors (5.58 KB, patch)
2004-03-12 03:34 PST, Darren Winsper
no flags Details | Diff | Splinter Review
Implement remaining CSS3 cursors (without -moz- prefix) (5.50 KB, patch)
2004-03-12 06:18 PST, Darren Winsper
no flags Details | Diff | Splinter Review
CSS3 cursor support with -moz- prefix (6.29 KB, patch)
2004-03-13 04:35 PST, Darren Winsper
no flags Details | Diff | Splinter Review
Updated patch as per lordpixel's comments (10.27 KB, patch)
2004-03-24 05:34 PST, Darren Winsper
lordpixel: review+
Details | Diff | Splinter Review
not-allowed for gtk/gtk2/xlib (288 bytes, image/png)
2004-07-29 18:19 PDT, Mats Palmgren (:mats)
no flags Details
col-resize for gtk/gtk2/xlib (161 bytes, image/png)
2004-07-29 18:21 PDT, Mats Palmgren (:mats)
no flags Details
row-resize for gtk/gtk2/xlib (169 bytes, image/png)
2004-07-29 18:22 PDT, Mats Palmgren (:mats)
no flags Details
vertical-text for gtk/gtk2/xlib (227 bytes, image/png)
2004-07-29 18:23 PDT, Mats Palmgren (:mats)
no flags Details
nesw-resize for gtk/gtk2/xlib (284 bytes, image/png)
2004-07-29 18:24 PDT, Mats Palmgren (:mats)
no flags Details
nwse-resize for gtk/gtk2/xlib (242 bytes, image/png)
2004-07-29 18:25 PDT, Mats Palmgren (:mats)
no flags Details
Proposed glyphs for gtk/gtk2/xlib on different backgrounds (1.98 KB, text/html)
2004-07-29 18:31 PDT, Mats Palmgren (:mats)
no flags Details
Testcase (4.28 KB, text/html)
2004-07-30 15:40 PDT, Mats Palmgren (:mats)
no flags Details
Patch B rev. 1 (147.74 KB, patch)
2004-07-30 17:31 PDT, Mats Palmgren (:mats)
no flags Details | Diff | Splinter Review
Patch B rev. 1 (diff -uw) (76.89 KB, patch)
2004-07-30 17:32 PDT, Mats Palmgren (:mats)
no flags Details | Diff | Splinter Review
Microsoft Windows custom cursor (.CUR) for CSS3 vertical-text (326 bytes, image/x-mswindows-cursor)
2004-07-31 00:31 PDT, Sergey «Mithgol the Webmaster» Sokoloff
no flags Details
Microsoft Windows custom cursor (.CUR) for CSS3 row-resize (326 bytes, image/x-mswindows-cursor )
2004-07-31 01:05 PDT, Sergey «Mithgol the Webmaster» Sokoloff
no flags Details
Microsoft Windows custom cursor (.CUR) for CSS3 col-resize (326 bytes, image/x-mswindows-cursor)
2004-07-31 01:21 PDT, Sergey «Mithgol the Webmaster» Sokoloff
no flags Details
Patch B rev. 2 (149.26 KB, patch)
2004-07-31 05:24 PDT, Mats Palmgren (:mats)
lordpixel: review-
Details | Diff | Splinter Review
Patch B rev. 2 (diff -uw) (78.42 KB, patch)
2004-07-31 05:26 PDT, Mats Palmgren (:mats)
no flags Details | Diff | Splinter Review
Patch for Camino cursor implementation (6.84 KB, patch)
2004-07-31 14:56 PDT, Andrew Thompson
no flags Details | Diff | Splinter Review
Additional tiff cursors for camino -> place in widget/src/cocoa/cursors/ to test (24.72 KB, application/octet-stream)
2004-07-31 15:03 PDT, Andrew Thompson
no flags Details
Added patch to Camino.xcode project file to add the new images (12.34 KB, patch)
2004-07-31 21:07 PDT, Andrew Thompson
no flags Details | Diff | Splinter Review
Patch B rev. 3 (156.28 KB, patch)
2004-08-01 06:50 PDT, Mats Palmgren (:mats)
no flags Details | Diff | Splinter Review
Patch B rev. 3 (diff -uw) (87.40 KB, patch)
2004-08-01 06:51 PDT, Mats Palmgren (:mats)
no flags Details | Diff | Splinter Review
Latest Mac & Camino diffs (29.24 KB, patch)
2004-08-12 19:11 PDT, Andrew Thompson
no flags Details | Diff | Splinter Review
Cursor .tiffs -> widget/src/cocoa/cursors/ (7.97 KB, application/octet-stream)
2004-08-12 19:19 PDT, Andrew Thompson
no flags Details
Patch B rev. 4 (178.67 KB, patch)
2004-08-15 18:29 PDT, Mats Palmgren (:mats)
dbaron: superreview+
Details | Diff | Splinter Review
Patch B rev. 4 (diff -uw) (111.02 KB, patch)
2004-08-15 18:31 PDT, Mats Palmgren (:mats)
no flags Details | Diff | Splinter Review
Handle compiling with 10.2 SDK (7.40 KB, patch)
2004-08-26 19:43 PDT, Andrew Thompson
no flags Details | Diff | Splinter Review
Patch B rev. 5 (177.49 KB, patch)
2004-08-27 13:19 PDT, Mats Palmgren (:mats)
no flags Details | Diff | Splinter Review
Patch B rev. 6 (177.04 KB, patch)
2004-08-31 18:31 PDT, Mats Palmgren (:mats)
mikepinkerton: review+
Details | Diff | Splinter Review
standard column resize (326 bytes, image/x-icon)
2004-09-13 02:08 PDT, neil@parkwaycc.co.uk
no flags Details
alternate column resize (326 bytes, image/x-icon)
2004-09-13 02:12 PDT, neil@parkwaycc.co.uk
no flags Details
alternate row resize (326 bytes, image/x-icon)
2004-09-13 02:13 PDT, neil@parkwaycc.co.uk
no flags Details
Patch C rev. 1 (2.41 KB, patch)
2004-09-18 09:26 PDT, Mats Palmgren (:mats)
dbaron: superreview+
Details | Diff | Splinter Review

Description Tim Powell 2002-08-16 19:17:34 PDT
Although CSS3 is not finalized, it would be quite helpful to support the
additional proposed cursor styles, especially since many of them would be
helpful to use internally in Mozilla. It would also provide needed capabilities
for developers that are creating web-applications.

The current CSS3 draft spec adds the following cursors to those supported in CSS2:
progress | all-scroll | col-resize | row-resize | no-drop | not-allowed |
vertical-text | copy | alias | context-menu | cell 

IE6 already supports many of these: 
progress | all-scroll | col-resize | row-resize | no-drop | not-allowed |
vertical-text
See the microsoft description of its cursor support:
http://msdn.microsoft.com/workshop/author/dhtml/reference/properties/cursor.asp
and
http://webdesign.about.com/library/weekly/aa081501a.htm

Mozilla needs the col-resize and row-resize cursors for column controls and
frame resizing - see bug 18958. I suspect Mozilla already supports the progress
cursor as well as the no-drop cursor. It also supports a dragging text selection
cursor that isn't mentioned in the spec, but could be exposed.

These additional cursors would be somewhat less necessary if Mozilla implemented
handling of URI values on CSS2 cursor properties as described in bug 38447, but
they'd be more convenient and helpful. It would also be nice if web developers
could rely on the IE and Mozilla to have similar cursors available.

An earlier draft for CSS3 cursors
http://www.w3.org/TR/2000/WD-css3-userint-20000216#cursor
also mentioned the grab and grabbing cursors that are asked for in bug 121986.
Personally, I think these are much better than the move cursor commonly used in
DHTML and should be included in CSS3 and Mozilla as well.

I appears that to fix this requires changes to the nsCursor identifier:
http://lxr.mozilla.org/seamonkey/ident?i=nsCursor
Comment 1 Sam J. Fleet 2002-08-16 19:32:51 PDT
Sound good to me!
Comment 2 Tim Powell 2002-08-16 21:23:12 PDT
An initial testcase is available:
http://www.worldtimzone.com/mozilla/testcase/css3cursors.html
Comment 3 Boris Zbarsky [:bz] (TPAC) 2002-09-08 14:13:06 PDT
Are the cursors in question actually supported natively on the mozilla platforms
(Win32, Mac, X, OS/2)?  Or would we need to start painting the cursor ourselves
or something ridiculous like that?
Comment 4 Sergey «Mithgol the Webmaster» Sokoloff 2002-12-27 05:06:27 PST
Mass voting for advance in CSS cursor implementation...
Comment 5 Tim Powell 2003-07-14 14:35:36 PDT
I've updated the testcase to include the known -moz cursors and to include more
pictures. Mozilla 1.4 apparently supports some of the CSS3 cursors, if not by
their actual identifiers:

CSS3 cursor       Mozilla equivalent
progress          -moz-spinning
copy              -moz-copy
alias             -moz-alias
cell              -moz-cell
context-menu      -moz-context-menu (mac/unix only?)

Unfortunately only the first overlaps with the CSS3 cursors supported by IE6.
Comment 6 Gérard Talbot 2004-01-07 10:51:48 PST
>Comment #3: Are the cursors in question actually supported natively on the
mozilla platforms (Win32, Mac, X, OS/2)?  
I don't see any CSS3 cursors (currently supported and rendered by MSIE 6) in the
Windows/Cursors directory. I suspect it is the case since Mozilla uses the
default selected text dragging cursor in Win32 for dragging a bookmark and so
does MSIE 6 when dragging a favorite on the Links toolbar.

>Or would we need to start painting the cursor ourselves or something ridiculous
like that?
If Mozilla needs to create, build some CSS3 cursors for the Win32 platforms (in
.cur - bitmap format), then I can do it in 32x32 bitmaps without problems. I
already started doing so at
http://www10.brinkster.com/doctorunclear/HTMLJavascriptCSS/Cursors.html
and anyone using MSIE 6 for Windows can see how the cursors look like; others
will just see the equivalent .gif.
Comment 7 Andrew Thompson 2004-02-07 08:28:08 PST
Since this is in Last Call now at the w3c, now would be a good time.

I can do the CSS3 parts of this, and the Mac OS implementation.

Not assigning it to myself just yet: I need to land some other cursor work first.
Comment 8 Darren Winsper 2004-03-12 03:32:43 PST
I've somewhat implemented the remaining CSS3 cursors since Glazman needed one of
 them implementing a few days ago.  I've prefixed all the keywords with "-moz-"
since the spec isn't final yet.

The patch I'm about to submit performs all the modifications necessary, adds the
extra cursors to the Gtk2 nsWindow class and implements not-allowed in the
Windows nsWindow class.  Some of the Gtk2 pointers aren't entirely appropriate
for the cursor type, so we may have to come up with our own.
Comment 9 Darren Winsper 2004-03-12 03:34:17 PST
Created attachment 143706 [details] [diff] [review]
Implement remaining CSS3 cursors
Comment 10 Andrew Thompson 2004-03-12 05:49:54 PST
Cool. I'll try to review your patch this weekend.
Once I get done with another cursor related bug, I'll do the Mac & Camino
implementations of the new cursors.
Comment 11 Anne (:annevk) 2004-03-12 05:57:49 PST
CSS3 UI is last call and can become a CR anytime; can't we just implement them
without the -moz- prefix?
Comment 12 Darren Winsper 2004-03-12 06:01:26 PST
The CSS3 cursors that Mozilla has already implemented have the -moz prefix, so
this path would keep them consistent.  However, changing the patch to not use
the prefix would be pretty trivial and I'll do it if people feel it's the right
thing to do.
Comment 13 Andrew Thompson 2004-03-12 06:07:54 PST
Actually I'd prefer that too. Its extremely unlikely anything will change now. I
thought it was appropriate for there to be sample implementations at this stage?
Comment 14 Darren Winsper 2004-03-12 06:18:56 PST
Created attachment 143709 [details] [diff] [review]
Implement remaining CSS3 cursors (without -moz- prefix)
Comment 15 Darren Winsper 2004-03-12 06:20:47 PST
OK, here's a new patch with the changes needed.  Perhaps I should add support
for the other cursors without -moz too.
Comment 16 Anne (:annevk) 2004-03-12 06:25:52 PST
> OK, here's a new patch with the changes needed.  Perhaps I should add support
> for the other cursors without -moz too.

That would be nice. That way everything is consistent. Officially we should drop
-moz- at CR stage (then they start looking for implementations and create a
suitable test case), however, this draft isn't going to change anymore.
Comment 17 Darren Winsper 2004-03-12 06:34:40 PST
I'll put support for the other cursors into the patch tomorrow, but I'm
reluctant to drop the -moz ones since they're used all over the place, so I'll
just make them equivalent.
Comment 18 Anne (:annevk) 2004-03-12 06:49:11 PST
After this bug has been fixed (new cursors and older cursors available without
-moz- prefix) there should be a new bug (or 2?) for cleanup:

 * Removing all the extra code for the cursors with the -moz- prefix.
 * Make sure that all style sheets use the syntax without the prefix (because
   it will be nog longer supported with the prefix).
Comment 19 Hixie (not reading bugmail) 2004-03-12 13:21:32 PST
No. Don't remove the -moz- prefix until it is in CR. It's a simple rule.

Too many times we've been burnt by people who thought "there's no chance this
can change before it reaches CR".

If you think it'll reach CR in a few days, and don't want to check in with -moz-
prefixes, then wait a few days. Otherwise, use the -moz- prefixes. For what it's
worth, I understand Tantek _has_ made some changes to the cursors in the draft
that will be going to CR. You can never know until it is actually published.
Comment 20 Darren Winsper 2004-03-13 04:35:24 PST
Created attachment 143786 [details] [diff] [review]
CSS3 cursor support with -moz- prefix
Comment 21 Darren Winsper 2004-03-13 04:37:19 PST
As per Hixie's point, I've reverted the patch to use -moz- prefixes.  I've also
added nesw_resize and nwse_resize, which I missed in the last patch.
Comment 22 Andrew Thompson 2004-03-13 06:48:27 PST
Even after it goes into CR the -moz cursors which have been included for years
should be left in. You can take out the ones you add this week, but things like
-moz-spinning are going to be used all over the place (Mozilla Extensions and
possibly even people's web pages). No point in breaking all of those uses.
Comment 23 Andrew Thompson 2004-03-13 07:41:58 PST
The core functionality of this patch looks good, but there are several issues to
address before it can be checked in:

* Standard format please. Typically I generate patches with

cvs diff -u -2 <filename> >> CSS3Cursors.patch

You can diff against cvs-mirror. You don't need access to the checkin server.

* You're still missing 2 constants from CSS3

ns-resize & ew-resize

* in windows/nsWindow.cpp, I see this:

+    case eCursor_not_allowed:
+      newCursor = ::LoadCursor(NULL, IDC_NO);
+      break;
+
     default:
       NS_ASSERTION(0, "Invalid cursor type");
       break;
Do we want to hit that assertion every time? I see these lines above:

2490     case eCursor_context_menu:
2491     case eCursor_count_up:
2492     case eCursor_count_down:
2493     case eCursor_count_up_down:
2494       break;

It seems like each case you don't want to handle on Windows uses an explicit
"break" to catch it. I don't think its a serious problem, but we should be
consistent.

* I know you're OK on Mac OS because if we see a cursor we don't understand in 
the Mac version of nsWindow.cpp we just set the standard left-arrow cursor.
Ditto for Camino.

Address these comments and I should be able to r= this.
Comment 24 Brendan Eich [:brendan] 2004-03-13 13:23:22 PST
> cvs diff -u -2 <filename> >> CSS3Cursors.patch

-pu8 is better -- more context, and function/method name or label on @@ lines.

/be
Comment 25 Darren Winsper 2004-03-24 05:34:24 PST
Created attachment 144655 [details] [diff] [review]
Updated patch as per lordpixel's comments
Comment 26 Darren Winsper 2004-03-24 05:37:13 PST
Here's a new patch that addresses lordpixel's concerns about the old one.  For
the Windows cursors, I've simply added the enum values in without a cursor. 
Hopefully some Windows developer can perform the appropriate mappings, since I
don't develop on Windows.
Comment 27 Andrew Thompson 2004-05-05 20:36:28 PDT
I will take this and try to get it sr'd so I can check it in.
Comment 28 Andrew Thompson 2004-05-05 20:38:14 PDT
I will take this and try to get it sr'd so I can check it in.

Darren, thanks for the patch. If you'd rather own the bug then by all means take
it... otherwise I'll try to make sure this doesn't get ignored indefinately.
Comment 29 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2004-05-06 15:34:41 PDT
I think you should stub-out the new values in all widget ports, even if you
don't implement them.  It also seems like you could have better approximations
for a bunch of the missing ones using existing cursors on Windows (the NWSE,
etc. sizing ones, and probably others), and perhaps on other platforms.

Also, please note that the CSS parser really shouldn't be accepting any cursors
that we don't actually support, although it's less of an issue (although still
an issue) for -moz-* cursors.
Comment 30 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2004-05-06 15:40:27 PDT
(In reply to comment #26)
> Hopefully some Windows developer can perform the appropriate mappings, since I
> don't develop on Windows.

That doesn't mean you can't read
http://msdn.microsoft.com/library/en-us/winui/winui/windowsuserinterface/resources/cursors/cursorreference/cursorfunctions/loadcursor.asp
http://developer.apple.com/documentation/Carbon/Reference/Appearance_Manager/appearance_manager/constant_14.html#//apple_ref/doc/c_ref/ThemeCursor
http://developer.gimp.org/api/2.0/gdk/gdk-cursors.html
etc., and make an attempt at a patch.
Comment 31 Andrew Thompson 2004-05-06 17:22:01 PDT
Thanks for the links.
Actually, I wrote the current cursor code for Mozilla Mac & Camino, so what I'd
planned to do is enter two more bugs for the actual implementation on those
platforms.
I feel we've got a 'good enough' on Windows & Linux. I can check all of the
other ports to see what they'll do if presented with a cursor that they don't
recognize.

I would do the extra cursors on Windows & Linux, but I don't have either around
to compile and test on, so I can't really :( I'll try if you insist, but it'd be
easier to file defects for those 2 platforms and try to get people who actually
have Windows/Linux boxes to own them.

I'd like to get the style system stuff in as it stands now, then start on the
Mac stuff. What do you think?
Comment 32 Darren Winsper 2004-05-06 17:31:12 PDT
I'm loathe to really touch the Windows and Mac cursor code since I don't have
any developer tools for Windows and I don't have any access to a Mac.  If I made
a change that broke Windows/Mac, I wouldn't know and wouldn't be able to fix it.

I'd be happy to help with the Linux implementation, but like I said, the Gtk2
cursors don't really map that well to a good number of the CSS3 cursors.
Comment 33 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2004-05-06 17:53:08 PDT
You should stub out the case statements on all widget ports.  That's not negotiable.

I'd hesitate to call this "good enough" on Windows given that it implements only
one of the 10 added cursors, but whatever.

It's often easier to break other platforms by not changing their code than by
changing it in the obvious way.
Comment 34 Andrew Thompson 2004-05-06 18:33:10 PDT
I read the links you sent and I think I can add 3 or 4 couple more cursors on
Windows and maybe a couple on Linux. I'll repost the patch when I've had a
chance to work in those changes and have checked all of the other ports.
Comment 35 Anne (:annevk) 2004-05-11 23:20:08 PDT
CSS3 UI has now reached CR.

The new values can be implemented without the -moz- prefix. Note that we must be
backwards compatible with existing values. These need to behave the same for
with and without -moz- the prefix; if implemented according to the CSS3 UI module.
Comment 36 Andrew Thompson 2004-05-15 10:42:38 PDT
(In reply to comment #35)
> CSS3 UI has now reached CR.
> 
> The new values can be implemented without the -moz- prefix.

Excellent. I'm currently working on the Camino implementation of all of the cursors.

I'll ensure anything we previously had implemented as -moz still works.
Comment 37 Mats Palmgren (:mats) 2004-07-29 14:00:45 PDT
(In reply to comment #35)
> CSS3 UI has now reached CR.
> 
> The new values can be implemented without the -moz- prefix. Note that we must be
> backwards compatible with existing values. These need to behave the same for
> with and without -moz- the prefix; if implemented according to the CSS3 UI
> module.

Don't we just usually drop the old -moz- counterparts when moving to CSS3?
David?
Comment 38 Andrew Thompson 2004-07-29 17:13:29 PDT
> Don't we just usually drop the old -moz- counterparts when moving to CSS3?
> David?


Not for the cursors which have had a -moz prefix for a long time.
There will be CSS for XUL and also possibly HTML pages out there that use -moz
because some of these keywords have been in the source since 2002.

Comment 39 Mats Palmgren (:mats) 2004-07-29 18:19:54 PDT
Created attachment 154715 [details]
not-allowed for gtk/gtk2/xlib
Comment 40 Mats Palmgren (:mats) 2004-07-29 18:21:26 PDT
Created attachment 154716 [details]
col-resize for gtk/gtk2/xlib
Comment 41 Mats Palmgren (:mats) 2004-07-29 18:22:27 PDT
Created attachment 154717 [details]
row-resize for gtk/gtk2/xlib
Comment 42 Mats Palmgren (:mats) 2004-07-29 18:23:31 PDT
Created attachment 154718 [details]
vertical-text for gtk/gtk2/xlib
Comment 43 Mats Palmgren (:mats) 2004-07-29 18:24:38 PDT
Created attachment 154719 [details]
nesw-resize for gtk/gtk2/xlib
Comment 44 Mats Palmgren (:mats) 2004-07-29 18:25:38 PDT
Created attachment 154720 [details]
nwse-resize for gtk/gtk2/xlib
Comment 45 Mats Palmgren (:mats) 2004-07-29 18:31:26 PDT
Created attachment 154721 [details]
Proposed glyphs for gtk/gtk2/xlib on different backgrounds
Comment 46 Andrew Thompson 2004-07-29 19:12:00 PDT
Next time you need to attach a lot, just zip or tar.gz please!
Comment 47 Mats Palmgren (:mats) 2004-07-30 15:40:31 PDT
Created attachment 154812 [details]
Testcase
Comment 48 Mats Palmgren (:mats) 2004-07-30 17:28:38 PDT
Taking bug, patch coming up...
Comment 49 Mats Palmgren (:mats) 2004-07-30 17:31:32 PDT
Created attachment 154824 [details] [diff] [review]
Patch B rev. 1

This patch adds the CSS3 cursors and also cleans up the naming
to match the CSS names. I have also fixed a problem in ESM who
mapped both NS_STYLE_CURSOR_N_RESIZE/S_RESIZE to eCursor_sizeNS
which made it impossible to differentiate these in the widget
classes. The are now propagated as eCursor_n_resize/s_resize.
(Same for the w/e case). I have added custom glyphs for
gtk/gtk2/xlib from the previously attached images.
If anyone can produce .cur versions for Windows for col-resize,
row-resize and vertical-text that would be great.
I have also stubbed out all the other widget sets and added
reasonable values, but of course there are some glyphs still
missing. I have only tested on gtk2, so please test and
report problems if you can for the other platforms.
Comment 50 Mats Palmgren (:mats) 2004-07-30 17:32:58 PDT
Created attachment 154825 [details] [diff] [review]
Patch B rev. 1 (diff -uw)
Comment 51 Sergey «Mithgol the Webmaster» Sokoloff 2004-07-31 00:31:16 PDT
Created attachment 154844 [details]
Microsoft Windows custom cursor (.CUR) for CSS3 vertical-text
Comment 52 Sergey «Mithgol the Webmaster» Sokoloff 2004-07-31 01:05:56 PDT
Created attachment 154845 [details]
Microsoft Windows custom cursor (.CUR) for CSS3 row-resize
Comment 53 Sergey «Mithgol the Webmaster» Sokoloff 2004-07-31 01:21:20 PDT
Created attachment 154846 [details]
Microsoft Windows custom cursor (.CUR) for CSS3 col-resize
Comment 54 Sergey «Mithgol the Webmaster» Sokoloff 2004-07-31 01:35:21 PDT
Just made three MSWin cursor versions aiming to satisfy "that would be great"
request.

These cursors are painted using only two colors: transparent color and inverted
background. (That's similar to how standard Windows "I" text cursor is
rendered.) Hotspots are defined in the center of the central line segment.
(That's similar to where hotspot of standard Windows "I" text cursor is
located.) So I hope there cursors will fit into usual Windows style seamlessly.
(I've tried them in Control Panel --> Mouse --> Cursors several times while
debugging hotspots, just to be sure.)
Comment 55 Mats Palmgren (:mats) 2004-07-31 05:24:35 PDT
Created attachment 154850 [details] [diff] [review]
Patch B rev. 2

Added support for the three custom cursors for Windows. (Thanks Sergey!)
They should be saved as col_resize.cur row_resize.cur and vertical_text.cur
(in widget/src/build/res/).

Any volounteers for testing this on Windows?

Also, if someone can provide OS/2 versions of the same (.ptr format) I'll
be glad to add those in as well.
Comment 56 Mats Palmgren (:mats) 2004-07-31 05:26:05 PDT
Created attachment 154851 [details] [diff] [review]
Patch B rev. 2 (diff -uw)
Comment 57 Andrew Thompson 2004-07-31 14:56:33 PDT
Created attachment 154878 [details] [diff] [review]
Patch for Camino cursor implementation

Here's my implementation for Camino. Please merge with your patch (which looks
good) and repost. Please use cvs diff -puw8 ... it'll help me review if I get
more context in the diff.

I'll implement the standard Mac version for Firefox/Mozilla shortly, but it'll
take me some hours to test because I haven't build the seamonkey tree in some
time.

I'll also be attaching an updated test case and the new .tiff files required
for Camino.
Comment 58 Andrew Thompson 2004-07-31 15:03:12 PDT
Created attachment 154879 [details]
Additional tiff cursors for camino -> place in widget/src/cocoa/cursors/ to test
Comment 59 Andrew Thompson 2004-07-31 21:07:51 PDT
Created attachment 154893 [details] [diff] [review]
Added patch to Camino.xcode project file to add the new images
Comment 60 Mats Palmgren (:mats) 2004-08-01 06:50:43 PDT
Created attachment 154918 [details] [diff] [review]
Patch B rev. 3

Merged "Patch B rev. 2" with camino/cocoa changes from lordpixel@mac.com

We now have custom made cursors for the three major platforms and
reasonable values for the others as well.
Comment 61 Mats Palmgren (:mats) 2004-08-01 06:51:58 PDT
Created attachment 154919 [details] [diff] [review]
Patch B rev. 3 (diff -uw)
Comment 62 Mats Palmgren (:mats) 2004-08-08 09:15:56 PDT
Comment on attachment 154918 [details] [diff] [review]
Patch B rev. 3

I have now also tested this on Windows XP and it worked fine.
Comment 63 Andrew Thompson 2004-08-09 18:25:59 PDT
I have almost finished by review of the attachment.

I need to do a little more work on the camino/mac implementation, however
depending on how this goes I may seperate those issues out into a seperate bug
and r+ this.

I do have one question about the gtk implementation. I see we have a gtk and a
gtk2 implementation, fair enough. But inside the gtk implementation, there seems
to be duplicate, inconsistent code:

widget/src/gtk/nsWidget.cpp & widget/src/gtk/nsWindow.cpp

Why do these two files exist, and why don't the implementations agree?
Comment 64 Andrew Thompson 2004-08-10 21:08:52 PDT
Sorry for spam, but want to keep progress updated.
Mac & Camino changes inspired by Mats comments in patch are almost done.

One slight wrinkle: we have to support building (as well as running) the Mac
Firefox/Mozilla build on Jaguar (10.2) so I need to adjust my patch with a few
#defines to make sure I don't use unavailable features.

Its midnight though, so that'll have to wait for tomorrow.
Comment 65 Andrew Thompson 2004-08-12 19:11:18 PDT
Created attachment 155998 [details] [diff] [review]
Latest Mac & Camino diffs
Comment 66 Andrew Thompson 2004-08-12 19:19:07 PDT
Created attachment 156000 [details]
Cursor .tiffs -> widget/src/cocoa/cursors/ 

Here's the latest mac & camino diffs and the zip containing the required cursor
images.

Merge this into the patch and I'll r+ - I got done reading the previous
version.

Steps to apply:
1. apply attachment 155998 [details] [diff] [review]
2. checkin the .tiffs from the ZIP into widget/src/cocoa/cursors
3. delete widget/src/cocoa/cursors/contextMenu.tiff from CVS - its no longer
needed
Comment 67 Mats Palmgren (:mats) 2004-08-15 18:29:17 PDT
Created attachment 156225 [details] [diff] [review]
Patch B rev. 4

Merged Mac & Camino stuff (attachment 155998 [details] [diff] [review]) from lordpixel@mac.com
The changed files since "Patch B rev. 3" are:

widget/src/mac/nsMacWidget.r
widget/src/mac/nsWindow.cpp
widget/src/cocoa/nsCursorManager.mm
camino/Camino.xcode/project.pbxproj

Changes to other files are the same as they were in rev. 3
Comment 68 Mats Palmgren (:mats) 2004-08-15 18:31:06 PDT
Created attachment 156226 [details] [diff] [review]
Patch B rev. 4 (diff -uw)
Comment 69 Mats Palmgren (:mats) 2004-08-15 18:47:11 PDT
(In reply to comment #63)
> I do have one question about the gtk implementation. I see we have a gtk and
> a gtk2 implementation, fair enough. But inside the gtk implementation, there
> seems to be duplicate, inconsistent code:
> 
> widget/src/gtk/nsWidget.cpp & widget/src/gtk/nsWindow.cpp
> 
> Why do these two files exist, and why don't the implementations agree?

Fair question, to be honest - I don't know why it's designed the way
it is - or not designed perhaps ;-)
There is some code under gtk2/gtk/xlib that is *identical* or that
only differs in indentation that should be shared IMO.
(e.g. nsGtkCursors.h)

However, I would appreciate if we could ignore those issues in this
patch, since they are platform specific and redesigning that will
probably delay this bug being fixed. But, if you do see differences
that *appears to be errors*, specifically in the cursor code, then
please point them out and I will fix them.
Comment 70 Mats Palmgren (:mats) 2004-08-15 19:00:01 PDT
Comment on attachment 156225 [details] [diff] [review]
Patch B rev. 4

Pinkerton, please review the Mac/Camino/Cocoa stuff in this patch, including
the .tiff images attached above.
Comment 71 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2004-08-20 16:09:38 PDT
Comment on attachment 156225 [details] [diff] [review]
Patch B rev. 4

sr=dbaron.  But could somebody attach the testcase in comment 2, without the
images, to the bug, so we have a permanent copy of it?	(Or if there was
another testcase you were testing with, that as well...)
Comment 72 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2004-08-20 16:16:09 PDT
Mats, when checking this in (since I'm not sure if you've added binary files
with cvs before -- or for that matter any files):  when you cvs add a binary
file (as some images are), remember to use "cvs add -kb <file>" (and then commit
the files after they're added).

If you mess up, you can use "cvs admin -kb <file>" to change to binary and "cvs
admin -kkv <file>" to change back to the default.  However, this doesn't change
what's in the repository, so if you checked ina binary file with "kv" keyword
substitution instead of "b" keyword substitution from a platform that does
newline substutions (MacCVS or a Windows cvs that converts to DOS line endings),
you'd need to copy the files aside, update, replace the uncorrupted files, and
commit again.
Comment 73 Andrew Thompson 2004-08-23 06:23:33 PDT
Since regular Mozilla bugs don't allow an arbitrary number of reviewers on a
patch, and Mats therefore had to wipe out my review status to ask pinkerton to
review, I'll add r+ lordpixel@mac.com here in this comment for the record.
Comment 74 Mike Pinkerton (not reading bugmail) 2004-08-26 12:11:53 PDT
+#ifdef MAC_OS_X_VERSION_10_3
+      cursor = OnPantherOrLater() ? kThemeResizeUpCursor : 135; break;
+#else
+      cursor = 135; break;
+#endif        

note that we do release builds with the 10.2.X SDK so this will most certainly
be false even when running on panther. If you need the constant, you'll have to
pull it in manually. Maybe wrap the duplicate define in something so it doesn't
conflict with developers who build with the 10.3 SDK.

thanks for updating camino, btw :D
Comment 75 Andrew Thompson 2004-08-26 19:43:21 PDT
Created attachment 157111 [details] [diff] [review]
Handle compiling with 10.2 SDK

OK - so I changed the implementation to hardcode the cursor values for 10.3
when *compiling* on 10.2, and to use the resource cursors when *running* on
10.2 as before. People who compile on 10.3 will get the "real" constants.

Let me know if you have any questions.
Comment 76 Mats Palmgren (:mats) 2004-08-27 13:19:54 PDT
Created attachment 157187 [details] [diff] [review]
Patch B rev. 5

Integrated latest widget/src/mac/nsWindow.cpp patch (no other changes to rev.
4)
Comment 77 Mats Palmgren (:mats) 2004-08-27 13:25:52 PDT
Comment on attachment 157187 [details] [diff] [review]
Patch B rev. 5

Link to the diff against "Patch B rev. 4":
http://bugzilla.mozilla.org/attachment.cgi?oldid=156225&action=interdiff&newid=
157187&headers=1
Comment 78 Mike Pinkerton (not reading bugmail) 2004-08-30 06:25:51 PDT
+//19, 20, 21 from Appearance.h in 10.3
+#define PANTHER_RESIZE_UP_CURSOR 19 
+#define JAGUAR_RESIZE_UP_CURSOR 135
+#define PANTHER_RESIZE_DOWN_CURSOR 20
+#define JAGUAR_RESIZE_DOWN_CURSOR 136
+#define PANTHER_RESIZE_UPDOWN_CURSOR 21
+#define JAGUAR_RESIZE_UPDOWN_CURSOR 141

how about placing the real constant name after the hardcoded value, but
commented out. also, i dislike using #defines for constants. use const short.
the compiler should do the right thing.

so it should look like, eg:

const short PANTHER_RESIZE_CURSOR = 19;  // kThemeResizeUpCursor
Comment 79 Mats Palmgren (:mats) 2004-08-31 16:41:55 PDT
Comment on attachment 157111 [details] [diff] [review]
Handle compiling with 10.2 SDK

>RCS file: /cvsroot/mozilla/widget/src/mac/nsWindow.cpp,v
>+#ifdef MAC_OS_X_VERSION_10_3
>+#define PANTHER_RESIZE_UP_CURSOR kThemeResizeUpCursor
>+#define JAGUAR_RESIZE_UP_CURSOR 135
>+#define PANTHER_RESIZE_DOWN_CURSOR kThemeResizeDownCursor
>+#define JAGUAR_RESIZE_DOWN_CURSOR 135

I'm assuming JAGUAR UP/DOWN should really be 135/136...
Comment 80 Mats Palmgren (:mats) 2004-08-31 18:31:58 PDT
Created attachment 157566 [details] [diff] [review]
Patch B rev. 6

Fixed comment 78 and comment 79.
I also removed the change to OnJaguarOrLater(), leaving it as it is in CVS now
-
it has been changed by bug 223545. I made a similar change to
OnPantherOrLater().

The only changed file since rev. 5 is: widget/src/mac/nsWindow.cpp
Comment 81 Andrew Thompson 2004-09-01 05:10:45 PDT
(In reply to comment #79)

> I'm assuming JAGUAR UP/DOWN should really be 135/136...


Yup. Amazing how many times one can read a piece of code and not see a typo.
The changes to onPantherOrLater()/onJaguarOrLater() look fine to me too. 

Comment 82 Mike Pinkerton (not reading bugmail) 2004-09-08 13:55:17 PDT
Comment on attachment 157566 [details] [diff] [review]
Patch B rev. 6

mac stuff looks ok r=pink
Comment 83 Mats Palmgren (:mats) 2004-09-11 19:24:22 PDT
Checked in to trunk at 2004-09-11 16:24 PDT.

There was a compile problem on "MacOSX Darwin 6.8 monkey" though:

/builds/tinderbox/SeaMonkey/Darwin_6.8_Depend/mozilla/widget/src/mac/nsWindow.cpp:78:
`
   kThemeResizeUpCursor' was not declared in this scope
/builds/tinderbox/SeaMonkey/Darwin_6.8_Depend/mozilla/widget/src/mac/nsWindow.cpp:79:
`
   kThemeResizeDownCursor' was not declared in this scope
/builds/tinderbox/SeaMonkey/Darwin_6.8_Depend/mozilla/widget/src/mac/nsWindow.cpp:80:
`
   kThemeResizeUpDownCursor' was not declared in this scope


In order to get the Tinderbox green I checked in the following "fix":

 #ifdef MAC_OS_X_VERSION_10_3
-const short PANTHER_RESIZE_UP_CURSOR     = kThemeResizeUpCursor;
-const short PANTHER_RESIZE_DOWN_CURSOR   = kThemeResizeDownCursor;
-const short PANTHER_RESIZE_UPDOWN_CURSOR = kThemeResizeUpDownCursor;
+const short PANTHER_RESIZE_UP_CURSOR     = 19;
+const short PANTHER_RESIZE_DOWN_CURSOR   = 20;
+const short PANTHER_RESIZE_UPDOWN_CURSOR = 21;
 #else

That is probably not correct, but I'm not familiar enough with these
different MacOSX versions to say what the correct code is.

Mike, Andrew - please advise what to do about that block.
Comment 84 Anne (:annevk) 2004-09-12 03:03:18 PDT
If try the test case, I don't get another cursor than the default by the
following values: 'context-menu', '-moz-context-menu', '-moz-count-up',
'-moz-count-down', '-moz-count-up-down'. Is that correct?

If that is correct, I suggest we remove support for those 4 -moz- values, unless
they are tested incorrectly of course.
Comment 85 Mats Palmgren (:mats) 2004-09-12 04:44:49 PDT
(In reply to comment #84)
> If try the test case, I don't get another cursor than the default by the

What platform and build?
The checked in code has not propagated to the nightly builds from mozilla.org
yet (at least not the Windows and Linux builds).
Comment 86 Anne (:annevk) 2004-09-12 06:59:40 PDT
2004091200, WinXP. All other values worked fine, so I guessed it was checked in.
(I'm using a build that is updated every few hours by the way, not the nightly.)
Comment 87 Mats Palmgren (:mats) 2004-09-12 07:17:16 PDT
OK, you're right, we have never had any glyph for -moz-context-menu on Windows.
I have spawned this off as bug 258960.
Comment 88 neil@parkwaycc.co.uk 2004-09-12 16:03:52 PDT
The column-resize cursor doesn't look like the version used by the windows list
view common control.

Also note that Windows style is to centre the hotspot if possible.
Comment 89 Sergey «Mithgol the Webmaster» Sokoloff 2004-09-12 17:30:32 PDT
(in reply to comment 88)

The col-resize hotspot is at (x=10; y=8) right now, isn't it? And overall width
is 21x17, so it seems centered well enough :-)

Meanwhile, may I have a look at the version used by the windows list view common
control? Is there an URL, or may you attach that here, or whatever else?
Comment 90 Sergey «Mithgol the Webmaster» Sokoloff 2004-09-12 18:01:50 PDT
I've played a little with several Windows applications right now.

Most of them (including Windows Explorer when navigating local filesystem in
Table view) use w-resize (e-resize) to resize columns as if that were windows.

Some applications use a simple bold cross with horizontal segment arrowed. It is
too primitive, so I have a strong feeling that attachment 154846 [details] should be
preferred, even if the bold cross is the above mentioned version used by the
windows list view common control.
Comment 91 neil@parkwaycc.co.uk 2004-09-13 02:08:13 PDT
Created attachment 158721 [details]
standard column resize

This is the cursor that I see when I hover over Explorer's Detail view columns.


P.S. forget my comment about centering, I was mistaken.
Comment 92 neil@parkwaycc.co.uk 2004-09-13 02:12:22 PDT
Created attachment 158722 [details]
alternate column resize

I subsequently discovered the existence of this cursor, which you may prefer.
Comment 93 neil@parkwaycc.co.uk 2004-09-13 02:13:46 PDT
Created attachment 158723 [details]
alternate row resize

Row resize version - mime type image/x-icon used to allow Mozilla to preview.
Comment 94 Mats Palmgren (:mats) 2004-09-18 09:26:31 PDT
Created attachment 159327 [details] [diff] [review]
Patch C rev. 1

I missed nsGlobalChromeWindow::SetCursor(). For completeness I think we
should support the new CSS3 cursor values here also. I'm also adding the
missing CSS2.1 value "progress".
Comment 95 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2004-09-18 10:10:16 PDT
Comment on attachment 159327 [details] [diff] [review]
Patch C rev. 1

If this is the same list, why not use the same code?  (That probably requires
moving nsCSSParser::SearchKeywordTable into nsCSSProps, making it return the
result instead of the index (which wouldn't hurt at all), and then calling
nsCSSKeywords::LookupKeyword and nsCSSProps::SearchKeywordTable (your new one,
which might need a different name from the existing ones, which I think should
be renamed to RSearch* -- and RSearchKeywordTableInt should return an
nsCSSKeyword enum instead of an int).)	But if you don't feel like doing that
now, this is fine...
Comment 96 Mats Palmgren (:mats) 2004-09-18 13:37:40 PDT
(In reply to comment #95)
> (From update of attachment 159327 [details] [diff] [review])
> If this is the same list, why not use the same code?

The thought occured to me but the list is not the same -
'grab*'/'spinning'/'count-*' are implemented here without the -moz- prefix.
(I filed bug 260272 on cleaning up this later)
Comment 97 Mats Palmgren (:mats) 2004-09-21 19:24:33 PDT
Comment on attachment 159327 [details] [diff] [review]
Patch C rev. 1

Bug 260272 has a better fix.
Comment 98 Gérard Talbot 2004-09-24 12:05:52 PDT
While using Mozilla 1.8a4 build 2004092404 under XP Pro SP2:
o col-resize, row-resize, not-allowed, vertical-text are correct.
o no-drop is the same as not-allowed.
o all-scroll is using the move cursor, not the autoscroll icon/cursor now in use
in Firefox.

http://www10.brinkster.com/doctorunclear/HTMLJavascriptCSS/Cursors.html#CSS3
Comment 99 Mats Palmgren (:mats) 2004-12-18 07:55:23 PST
I have spawned bug 275173 and bug 275174 to improve the no-drop/all-scroll
glyphs on Windows.

If there are other any other issues, please file new bugs.

I would like to thank all who contributed, Darren Winsper who did initial
work on this bug, Sergey "Mithgol the Webmaster" Sokoloff who did the Windows
cursor glyphs, Michael Kaply (IBM) who provided cursors for OS/2 and special
thanks to lordpixel@mac.com who did all the work for Mac.
Thanks to everyone else too for providing valuable feedback.

-> FIXED (remaining issues filed separately)
Comment 100 Sergey «Mithgol the Webmaster» Sokoloff 2004-12-18 11:41:12 PST
It was a pleasure to work with you.

And you may have already noticed my Windows glyph dropped inside the new bug 275173.

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