Closed Bug 163174 Opened 22 years ago Closed 20 years ago

[FIX] Support CSS3 cursors

Categories

(Core :: CSS Parsing and Computation, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: tpowellmoz, Assigned: MatsPalmgren_bugz)

References

(Depends on 2 open bugs, Blocks 1 open bug, )

Details

(Keywords: css3, testcase)

Attachments

(18 files, 17 obsolete files)

288 bytes, image/png
Details
161 bytes, image/png
Details
169 bytes, image/png
Details
227 bytes, image/png
Details
284 bytes, image/png
Details
242 bytes, image/png
Details
1.98 KB, text/html
Details
4.28 KB, text/html
Details
326 bytes, image/x-mswindows-cursor
Details
326 bytes, image/x-mswindows-cursor
Details
326 bytes, image/x-mswindows-cursor
Details
7.97 KB, application/octet-stream
Details
178.67 KB, patch
dbaron
: superreview+
Details | Diff | Splinter Review
111.02 KB, patch
Details | Diff | Splinter Review
177.04 KB, patch
mikepinkerton
: review+
Details | Diff | Splinter Review
326 bytes, image/x-icon
Details
326 bytes, image/x-icon
Details
326 bytes, image/x-icon
Details
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
Sound good to me!
An initial testcase is available:
http://www.worldtimzone.com/mozilla/testcase/css3cursors.html
Severity: normal → enhancement
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?
Mass voting for advance in CSS cursor implementation...
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 #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.
Keywords: css3
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.
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.
Attached patch Implement remaining CSS3 cursors (obsolete) — Splinter Review
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.
CSS3 UI is last call and can become a CR anytime; can't we just implement them
without the -moz- prefix?
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.
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?
Attachment #143706 - Attachment is obsolete: true
OK, here's a new patch with the changes needed.  Perhaps I should add support
for the other cursors without -moz too.
> 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.
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.
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).
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.
Attachment #143709 - Attachment is obsolete: true
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.
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.
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.
> cvs diff -u -2 <filename> >> CSS3Cursors.patch

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

/be
Depends on: 38447, 48839
Attachment #143786 - Attachment is obsolete: true
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.
Attachment #144655 - Flags: review+
Status: NEW → ASSIGNED
Attachment #144655 - Flags: superreview?(dbaron)
I will take this and try to get it sr'd so I can check it in.
Assignee: dbaron → lordpixel
Status: ASSIGNED → NEW
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.
Status: NEW → ASSIGNED
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.
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?
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.
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.
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.
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.
(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.
(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?
> 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.

Next time you need to attach a lot, just zip or tar.gz please!
Attached file Testcase
Taking bug, patch coming up...
Assignee: lordpixel → mats.palmgren
Status: ASSIGNED → NEW
Keywords: testcase
Summary: Support CSS3 cursors → [FIX] Support CSS3 cursors
Attached patch Patch B rev. 1 (obsolete) — Splinter Review
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.
Attached patch Patch B rev. 1 (diff -uw) (obsolete) — Splinter Review
Attachment #154824 - Flags: superreview?(dbaron)
Attachment #154824 - Flags: review?(lordpixel)
Attachment #144655 - Attachment is obsolete: true
Attachment #144655 - Flags: superreview?(dbaron)
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.)
Attached patch Patch B rev. 2 (obsolete) — Splinter Review
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.
Attachment #154824 - Attachment is obsolete: true
Attachment #154825 - Attachment is obsolete: true
Attached patch Patch B rev. 2 (diff -uw) (obsolete) — Splinter Review
Attachment #154824 - Flags: superreview?(dbaron)
Attachment #154824 - Flags: review?(lordpixel)
Attachment #154850 - Flags: superreview?(dbaron)
Attachment #154850 - Flags: review?(lordpixel)
Attachment #154850 - Flags: superreview?(dbaron)
Attachment #154850 - Flags: review?(lordpixel)
Attachment #154850 - Flags: review-
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.
Attachment #154850 - Attachment is obsolete: true
Attachment #154851 - Attachment is obsolete: true
Attachment #154878 - Attachment is obsolete: true
Attached patch Patch B rev. 3 (obsolete) — Splinter Review
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.
Attachment #154893 - Attachment is obsolete: true
Attached patch Patch B rev. 3 (diff -uw) (obsolete) — Splinter Review
Attachment #154918 - Flags: review?(lordpixel)
Comment on attachment 154918 [details] [diff] [review]
Patch B rev. 3

I have now also tested this on Windows XP and it worked fine.
Attachment #154918 - Flags: superreview?(dbaron)
Flags: blocking1.8a3?
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?
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.
Attached patch Latest Mac & Camino diffs (obsolete) — Splinter Review
Attachment #154918 - Attachment is obsolete: true
Attachment #154919 - Attachment is obsolete: true
Attachment #154918 - Flags: superreview?(dbaron)
Attachment #154918 - Flags: review?(lordpixel)
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
Attachment #154879 - Attachment is obsolete: true
Attached patch Patch B rev. 4Splinter Review
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
Attachment #155998 - Attachment is obsolete: true
(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.
Attachment #156225 - Flags: review?(lordpixel)
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.
Flags: blocking1.8a3? → blocking1.8a3-
Attachment #156225 - Flags: review?(lordpixel) → review+
Attachment #156225 - Flags: superreview?(dbaron)
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...)
Attachment #156225 - Flags: superreview?(dbaron) → superreview+
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.
Attachment #156225 - Flags: review+ → review?(pinkerton)
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.
+#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
Attached patch Handle compiling with 10.2 SDK (obsolete) — Splinter Review
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.
Attached patch Patch B rev. 5 (obsolete) — Splinter Review
Integrated latest widget/src/mac/nsWindow.cpp patch (no other changes to rev.
4)
Attachment #157111 - Attachment is obsolete: true
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
Attachment #157187 - Flags: review?(pinkerton)
Attachment #156225 - Flags: review?(pinkerton)
+//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 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...
Attached patch Patch B rev. 6Splinter Review
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
Attachment #157187 - Attachment is obsolete: true
Attachment #157187 - Flags: review?(pinkerton)
Attachment #157566 - Flags: review?(pinkerton)
Flags: blocking1.8a4?
(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 on attachment 157566 [details] [diff] [review]
Patch B rev. 6

mac stuff looks ok r=pink
Attachment #157566 - Flags: review?(pinkerton) → review+
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.
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.
(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).
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.)
OK, you're right, we have never had any glyph for -moz-context-menu on Windows.
I have spawned this off as bug 258960.
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.
(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?
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.
Attached image 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.
Attached image alternate column resize
I subsequently discovered the existence of this cursor, which you may prefer.
Attached image alternate row resize
Row resize version - mime type image/x-icon used to allow Mozilla to preview.
Attached patch Patch C rev. 1 (obsolete) — Splinter Review
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".
Attachment #159327 - Flags: superreview?(dbaron)
Attachment #159327 - Flags: review?(neil.parkwaycc.co.uk)
Flags: blocking1.8a4?
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...
Attachment #159327 - Flags: superreview?(dbaron) → superreview+
(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 on attachment 159327 [details] [diff] [review]
Patch C rev. 1

Bug 260272 has a better fix.
Attachment #159327 - Attachment is obsolete: true
Attachment #159327 - Flags: review?(neil.parkwaycc.co.uk)
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
Depends on: 275173
Depends on: 275174
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)
Status: NEW → RESOLVED
Closed: 20 years ago
Depends on: 258966, 260713
Resolution: --- → FIXED
It was a pleasure to work with you.

And you may have already noticed my Windows glyph dropped inside the new bug 275173.
Blocks: 280331
Depends on: 204841
No longer depends on: 204841
You need to log in before you can comment on or make changes to this bug.