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)

(deleted), image/png
Details
(deleted), image/png
Details
(deleted), image/png
Details
(deleted), image/png
Details
(deleted), image/png
Details
(deleted), image/png
Details
(deleted), text/html
Details
(deleted), text/html
Details
(deleted), image/x-mswindows-cursor
Details
(deleted), image/x-mswindows-cursor
Details
(deleted), image/x-mswindows-cursor
Details
(deleted), application/octet-stream
Details
(deleted), patch
dbaron
: superreview+
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
mikepinkerton
: review+
Details | Diff | Splinter Review
(deleted), image/x-icon
Details
(deleted), image/x-icon
Details
(deleted), 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!
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) (deleted) — 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.
Attached patch CSS3 cursor support with -moz- prefix (obsolete) (deleted) — Splinter Review
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
Attached patch Updated patch as per lordpixel's comments (obsolete) (deleted) — Splinter Review
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.
Attached image not-allowed for gtk/gtk2/xlib (deleted) —
Attached image col-resize for gtk/gtk2/xlib (deleted) —
Attached image row-resize for gtk/gtk2/xlib (deleted) —
Attached image vertical-text for gtk/gtk2/xlib (deleted) —
Attached image nesw-resize for gtk/gtk2/xlib (deleted) —
Attached image nwse-resize for gtk/gtk2/xlib (deleted) —
Next time you need to attach a lot, just zip or tar.gz please!
Attached file Testcase (deleted) —
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) (deleted) — 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) (deleted) — 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) (deleted) — 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) (deleted) — 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-
Attached patch Patch for Camino cursor implementation (obsolete) (deleted) — Splinter 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) (deleted) — 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) (deleted) — 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) (deleted) — 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. 4 (deleted) — Splinter 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
Attached patch Patch B rev. 4 (diff -uw) (deleted) — Splinter Review
(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) (deleted) — 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) (deleted) — 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. 6 (deleted) — Splinter 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 (deleted) —
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 (deleted) —
I subsequently discovered the existence of this cursor, which you may prefer.
Attached image alternate row resize (deleted) —
Row resize version - mime type image/x-icon used to allow Mozilla to preview.
Attached patch Patch C rev. 1 (obsolete) (deleted) — 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.

Attachment

General

Creator:
Created:
Updated:
Size: