This bug has been broken off from bug #1916, which is now a meta bug about CSS2/3 cursor property support.

Hmmm... what cursor formats should be accepted? Are there any good XP cursor formats?

SVG?

Resolved as Later. It raises about the same problems as downloadable fonts and has nowhere near the same usefulness.

By which of course, you meant "Milestone: Future". Right?

My Latered bugs are meant to be reconsidered after we ship this version. I think it's what Future is for too but I prefer Later because the bugs don't appear on top of my list. I understand that it can make them harder to find but I don't have a lot of them and I don't think I ever received a dup for any of these, so I prefer to mark them Later and avoid the noise. I may change my habits if bug 38477 gets fixed (or if you give me a compelling reason :-). Re-latered for now.

Netscape's standard compliance QA team reorganised itself once again, so taking remaining non-tables style bugs. Sorry about the spam. I tried to get this done directly at the database level, but apparently that is "not easy because of the shadow db", "plus it screws up the audit trail", so no can do...

Despite comments above, LATER is deprecated, so reopening and moving to Future.

Windows builds should support .cur and .ani, which are expected to be the majority of URI requests. Support on other platforms for these formats isn't expected, so it's safe to use Windows API.

Recognizing a platform-specific cursor format for web pages (which should be platform-independent) is a Bad Thing (TM).

dbaron: What platform-independent cursor format are you thinking of, then?

PNG with a custom hotspot block would do the trick. dbaron: Of course, we could `just' implement a CUR/ANI decoder for the other platforms, so then it would be XP. ;-)

Ahh, but creating browser-proprietary formats is A Much More Bad Thing! If IE6 supports one format (.CUR/.ANI, basically the same thing), and NS6 supports another, we'll just be back to the old days of incompatibility between browsers. Until W3C creates a standardized cursor format, it would be best just to stick with .CUR and .ANI. I don't think any webmasters will be tolerant enough of Netscape to use a proprietary format just so they can style the cursor (as it is lots of sites block out NS6 and send NS4 to a text-only page).

And in case it wasn't clear from Skewer's comments, IE6 will support .cur and .ani. Read all about it: http://msdn.microsoft.com/workshop/author/dhtml/reference/properties/cursor.asp? frame=true Mozilla should support .cur and .ani as well, since they'll undoubtably be the most common format, especially if IE6 gets there with support first. Microsoft faces the same XP problem with the Mac version of IE. I wonder if it will support .cur and .ani? For that matter, how detrimental is it to not support the cursor URL syntax on all platforms? It seems like you could do something like cursor: url("yourcursor.cur"); cursor: pointer for browsers that don't support the url syntax. I'd suggest just using the Windows API and add XP support later. As hixie notes, it would be possible to implement a .cur or .ani decoder. I don't think those image formats are very complex.

Actually, the correct format is cursor: url("yourcursor.cur"), pointer; (broken, bug 77974). Question: Do MacOS and Linux have their own cursor formats? If so, it would make sense to support those as well in their respective versions.

Needs a dev relnote.

Macintosh cursors are traditionally embedded "crsr" Resources rather than standalone files. These Resources appear to contain a series of numbered bitmap graphic resources each of which has a single "hot" pixel. I suppose you could specify ".rsrc" file that contained a single "crsr" resource as the Mac OS (Classic) solution. I'm unaware of changes made for Mac OS X.

Actually, my comments are a little incomplete. See [http://developer.apple.com/techpubs/mac/QuickDraw/QuickDraw-401.html] for complete Mac OS cursor details.

I suppose that begs the question, "Can a URI refer to a resource within a Macintosh file, and if so, how?"

CCing the guys responsible for the BMP/icon decoder for bug 18502. They may be able to help with a cross-platform solution. It may be possible to load the bitmap cursor then reformat it for each platform's own cursor format--and maybe eventually vice-versa. We should avoid displaying the cursor as a dynamically drawn bitmap at all costs, even if it means not supporting it. With all this talk of decoding cursor formats some may suggest creating a new special format for Mozilla. I'm still against this--nobody will use it unless it appears somewhere at W3C or is in use in a popular operating system (what does Linux use?), and the existing formats would have no hotspot, severely limiting their use. If we tried, for example, a PNG-CUR conversion, that would just be a waste of space (and bloat the browser even more).

A .cur file is the same format as an .ico file, so we have a decoder for it.

In response to concerns about the Mac, we could support 2 types of cursors: - windows CUR file: the file can be converted to the binary representation of 'crsr' resource. It is basically a problem of conversion between Windows bitmaps and Mac bitmaps. - native 'crsr': the file should contain in its data fork the binary data of a 'crsr' resource. Then in both cases, the data can be dynamically added to the application's resource fork as a 'crsr' resource. For more info on the CUR format, see http://www.oreilly.com/centers/gff/formats/miccur/

*** Bug 114938 has been marked as a duplicate of this bug. ***

Assigning pierre's remaining Style System-related bugs to myself.

worth mentioning... the current W3C CSS 2.1 *Draft* no longer defines the uri property... nor does the CSS3 User Inferface *Draft*. http://www.w3.org/TR/2002/WD-CSS21-20020802/ui.html#cursor-props http://www.w3.org/TR/2000/WD-css3-userint-20000216.html#cursor I haven't taken the time to track down any relevant discussions on www-style so I don't know if this is an intentional move or simply an oversight in the current drafts.

To Comment 24: I don't understand what you're talking about? http://www.w3.org/TR/2003/WD-CSS21-20030128/ui.html#cursor-props http://www.w3.org/TR/CSS21/ui.html#cursor-props I can see the <uri> value specified. This one: http://www.w3.org/TR/2000/WD-css3-userint-20000216.html#cursor states "extensions to CSS2" and "New Values" only. And here: http://www.w3.org/TR/2002/WD-css3-ui-20020802/#cursor http://www.w3.org/TR/css3-ui/#cursor the <uri> presents too.

Apologies... I have been looking at an older 2.1 draft... so scratch my previous comments. Looking at the updated drafts I see mention of SVG cursors... does this give us any more direction as to a cross platform/standard cursor? http://www.w3.org/TR/SVG/interact.html#CursorElement

If there is a standard cross-platform cursor format, we should support it. But in the meantime it is absurd for Mozilla not to support a platform-dependent cursor file on its native platform (e.g. a .cur file). The reason CSS2 (and CSS3) supports repeated URI's is because you can provide a cursor file in each different format to ensure compatibility.

*** Bug 177193 has been marked as a duplicate of this bug. ***

I recommend that while everyone is arguing :-) someone should set it up so that if Mozilla cannot display the cursor, it would fall back to a specified cursor (like n-resize, crosshair, etc.) sort of like alternate text for images. Example CSS to get that effect: .cursor { cursor: crosshair; } .cursor { cursor: url(cool-cursor.cur); } I would hope that this makes the crosshair show up if the cool-cursor does not. If I could find where to edit (and if it does not require compiling) I would not mind at least tring to make this work. I am almost HTML/CSS guru, and have an idea what I am doing with Javascript.

comment 29 is the wrong syntax, but the right idea. See bug 77974.

Why don't we use formats already supported by Mozilla, like for the favicon (where .ico, PNG, GIF, MNG... are accepted) ?

Because Mozilla doesn't draw the cursor itself -- the OS draws the cursor.

stupid question: hiding os cursor and painting own cursor ?

if in CSS to write: ".aaa {cursor: hand;}" and then to use it in HTML as: <button class="aaa">bla bla bla</button> the mouse pointer is not done "hand" in Mozilla 1.6 :( Why?

That would be because 'hand' does not occur in any specification (but 'pointer' does, and is rendered as a hand on some OS) so Mozilla is fully conformant there.

For additional testing,see the SVG Test suite in particular http://www.w3.org/Graphics/SVG/Test/20030813/svggen/interact-cursor-01-f.svg linked from http://www.w3.org/Graphics/SVG/Test/20030813/htmlframe/full-interact-cursor-01-f.html SVG-enabled builds of Mozilla have partial support.

What exactly does this bug need to get fixed? A cross-platform .cur converter? I've got a textfile (of unknown origin) with specifications of MS Win BMP/ICO/CUR format. Is there any Mac's? Let's start that Windows <--> Macintosh thing.

Well, the issue of 'what format' was solved in SVG by inventing a cursor *element* which has an x and y hotspot as attributes, plus a link to a PNG file. Since all SVG implementations have to support PNG (and JPEG) then this gives enough info for a platform-independent cursor property (the cursor property points to url(url-of-cursor-element) which can be in a separate file of course. Pointing to platform-specific cursor files is harmful IMHO CSS3 takes the SVG idea and adds the hotspot x and y values right into the property definition, removing the need to indirect via an XML snippet. So what it takes to implement this, at least for SVG and for CSS3, is the ability to call whatever the platform has for a 'construct custom cursor from this image and this x and y data' function call.

> Pointing to platform-specific cursor files is harmful IMHO I tend to agree, though Internet Explorer already supports this property and has two different file formats: .cur/.ani (see also comment 12).

Yes, IE supports something like this property (although it calls pointer 'hand' despite the explanation that this is offensive in some cultures which I ,and the CSS WG, explained to them about 6,7 years ago ... its never been updated to CSS2 in the interim and its unlikely that it ever will since the project is disbanded. The broader point is that if MacoS X folks point toMacOS X cursor formats and Gnome and KDE folks point to X11 cursor formats and set-top-box fols point to set-top-box formats and PDA .... you get a bunch of platform specific content. Its way better to have cross-platform contentand put the platform-specific aspects (which function to call) in the implementation rather than exposed in the content.

IE6 support both 'hand' and 'pointer', but I was not talking about that. There are indeed cross-platform problems. And a PNG image in combination with a SVG would be nice. But I don't think authors are going to use that when it doesn't work in Internet Explorer. We could try to add support for SVG in combination with PNG first (leave alone that SVG is not enabled by default), but I think there should be a function/class that converts '.cur' or '.ani' into '.png'. This is probably not necessary for Mac and Unix cursors, since there isn't any browser yet on those platforms that has support for the 'cursor:url()'.

Chris: I ask out of curiosity: The disbanded project you mention is IE, isn't it? CSS2 is alive and kicking (new 2.1 spec being the evidence)

(yes the disbanded projects are IE/Win and IE/Mac, both officially listed as not under development) If we are limited to what IE supports then why bother with standards? If stuff looks 'ok' on legacy browsers (remember, url has a fallback to a predefined cursor shape) but better on newer, standards-based browsers then that s just a migration pressure, right? BTW there are of course implementations of custom cursors on MacOS and X11. Perhaps you meant 'arent any HTML browsers that support ...'. The SVG implementations do. For Mozilla, if the desire is to not introduce any dependency on SVG (despite Brendan Eich listing SVG as crucial for the development of Mozilla) then the CSS3 cursor property with the hotspot additions would suffice for pointing to a PNG without indirecting through SVG.

Bug 169678 may be related ["Cursor shape should reflect the type of link hovered on (e.g. pop-up/download/mailto...) (different cursors for links to new windows)"] Prog.

well personally, I'd say the way to go is to add a function to set any imgIContainer as a cursor, and implement decoders for .cur (I believe we have that, except hotspot information (via the .ico decoder))/.ani/macos's cursor format. > function/class that converts '.cur' or '.ani' into '.png' why would that be desirable?

My name is being invoked bogusly. Anyone with experience managing software by requirements knows you don't turn on SVG in the default builds at this point to fix this bug. SVG's day will come; the day when we don't have to care about what IE supports may come, but I doubt it. Realism, please. /be

> why would that be desirable? That or a similar function to create a 'bridge' accross different platforms. If we can simple encode them it is fine with me. I was just suggesting something Mozilla could do, to show it on every platform. When can we expect SVG? I think that Mozilla should support at least the CSS3 solution, since that is probably a lot easier to understand than SVG (not probably, it is).

As I said in comment 32, I think the hard part would be the "function to set any imgIContainer as a cursor". Though it might not be as bad as I'd think. If somebody plans to do this across platforms, I could write a patch to upgrade our 'cursor' parsing to css3-ui ( http://www.w3.org/TR/css3-ui/#cursor ), which has the syntax: [ [<uri> [<x> <y>],]* <keyword> | inherit ] instead of just [ [<uri>,]* <keyword> | inherit ] and propagate both the URIs and the coordinates through the style system.

(In reply to comment #48) > As I said in comment 32, I think the hard part would be the "function to set any > imgIContainer as a cursor". I didn't mean to imply that it was easy :) (In reply to comment #47) > > why would that be desirable? > > That or a similar function to create a 'bridge' accross different platforms. I was asking because I doubt any platform uses png as their native cursor format... it seems so unlikely that toolkit apis require a compressed format.

Clearly most API calls would require uncompressed image data. Decoding the PNG is not that hard, though ;-) Wrapping allthe platform-specific apis with a platform independent api that took a PNG and hotspot would be reasonable.

"When can we expect SVG?" well, its already on the trunk, so just compile it up if you are curious. SVG-enabled Mozilla passes all of the cursor test except for the custom, URI-defined cursor subtest.

Brendan said "My name is being invoked bogusly." No, I just mentioned exactly what your slides said, in passing. "Anyone with experience managing software by requirements knows you don't turn on SVG in the default builds at this point to fix this bug." Correct. You turn it on because people want the functionality, and it gives them a reason to upgrade their browser to a good, modern, standards compliant one. It also, btw, would give a second reason to fix this bug (note that the SVG builds *also* have this bug for the exact same reason). "SVG's day will come" well it would come a lot quicker if people got the functionality built in and could use it everywhere (CSS backgrounds, chrome, etc) instead of being limited to what a plugin can do. "the day when we don't have to care about what IE supports may come, but I doubt it." Chuckle. If you want to lock yourself into some four year old, bugwards compatible definition of reality then feel free. Mozilla as a whole has to decide if it wants to use the time between the cancellation of IE and the shipment of Longhorn to grab market share or to tiptoe around being careful not to give anyomne a reason to switch. "Realism, please." My thoughts exactly. We seem to have a different view of what is realistic and desirable. It has been very refreshing to see the rapid advances in XHTML, SMIL, SVG compliance of the cellphone market which is untrammelled by the bugwars compatible morass. Hopefully the desktop market, stagnant for years, will catch up at some point.

(In reply to comment #50) > Wrapping allthe platform-specific apis with a platform independent api that took > a PNG and hotspot would be reasonable. An imgIContainer as I suggested works just fine for that, without complexifying it by a specific image format...

chris@w3.org, please stop spamming this bug. Trying to attach glory and realism to yourself from some other peoples' achievements in cell-phone browsers has no place in this bug, and no relevance to desktop browsers that have to deal with the bulk of the web-as-it-is, not the web-as-you-wish-it-were, or the-tiny-cell-phone-targeted-subset of the web. Oh, wait -- you're telling me those cell-phone-targeted SVGT pages work in IE? I didn't think so. Making something work in Mozilla, but not in Safari and Opera, and probably in IE via a plugin or extension, does almost nothing to make that something succeed on the web. It can actually be a negative, due to opportunity costs. Mozilla is in jeopardy of losing several current "sales" (not that we make any money, necessarily) precisely because we do not support IE proprietary crap. That's reality, I face it every day. I suggest that you are in an ivory tower, namely, the w3c. Try developing a desktop browser and getting distribution channels for it some time -- your casual opinion of others' realism may change. A lot. I'm done here. /be

(In reply to comment #3) > It raises about the same problems as downloadable fonts and > has nowhere near the same usefulness. Custom cursors can be very useful if the user wants to make certain types of elements easily recognizable via userContent.css (I ran into this bug when I tried to set up a special cursor for links that open new windows). You don't need cross-platform support for this, just supporting the native cursor format of the underlying OS would be sufficient.

This is the file I mentioned three months ago, in comment 37. I feel that this kind of specs will come handy in coding Win32 solution for conversion Mac <--> Win, or SVG <--> CUR, or CUR <--> imgIContainer. Unfortunately, I've got no ANI specs. And if someone is still curious about the file's origin: I've discovered it to be once UUE-decoded by me from some Fidonet echo, region R50. Still I don't know exactly, what echo was it. The file itself is dated May 1994. Wow, it's a really old thing! ;-)

nsGNOMEShellService.cpp has a way to convert an gfxIImageFrame into a GdkPixbuf. (WriteImage does that) With Gtk 2.4, you can set a GdkPixbuf as cursor: http://developer.gnome.org/doc/API/2.0/gdk/gdk-Cursors.html#gdk-cursor-new-from-pixbuf http://developer.gnome.org/doc/API/2.0/gdk/gdk-Windows.html#gdk-window-set-cursor .cur files are really just .ico files, and mozilla supports those. the only thing missing currently is the hotspot information, but that's easily addable.

interface changes for widget/ + stub impl in nsBaseWidget

this implements the CSS 2.1 syntax only. CSS 3 is left as an exercise to the reader :) I didn't know the CSS Parser well enough to know how to store the two coordinates as well on the nsCSSValue. This depends on the previous two attachments. There are a few unrelated string changes in there; I can separate them if desired.

I have an impl for gtk2 too. I don't want to attach it just yet though, it currently only works with gtk 2.4 (and prevents running mozilla on older versions). for this bug, I will do gtk2 and windows implementations, and file bugs for other platforms.

Comment on attachment 154944 [details] [diff] [review] imagelib part I'd really like to see how this is being used prior to r='ing this patch. I don't really like adding ICO specific parts to the imagelib APIs. Does anything else use this?

Why don't you like the idea? The resulting APIs (after the above patch applied) are still backwards compatible, aren't they? If they aren't, please quote a few codestrings proving their backward incompatibility. If they are, then what are you actually worried about?

If the CUR changes are approved, I can add the same functionality for XBM cursors. (Of course it would be nice if my other XBM changes are checked in first...)

OK.... pavlov, this shows how gtk2 would use this. win32 would use it in a similar way, for the X- and Y-hotspot members of http://msdn.microsoft.com/library/default.asp?url=/library/en-us/winui/WinUI/WindowsUserInterface/Resources/Icons/IconReference/IconStructures/ICONINFO.asp

(In reply to comment #63) > Why don't you like the idea? The resulting APIs (after the above patch applied) > are still backwards compatible, aren't they? why would that matter? (In reply to comment #62) > I >don't really like adding ICO specific parts to the imagelib APIs. Does >anything else use this? as ask said, XBM supports this too - http://netghost.narod.ru/gff/graphics/summary/xbm.htm#XBM-DMYID.2

Comment on attachment 154956 [details] [diff] [review] content/layout changes >Index: layout/base/public/nsIFrame.h >+ /** >+ * This structure holds information about a cursor. mContainer represents a >+ * loaded image that should be preferred. If it is not possible to use it, or >+ * if it is null, mCursor should be used. >+ */ >+ struct Cursor { >+ nsCOMPtr<imgIContainer> mContainer; >+ PRInt32 mCursor; >+ }; Is it true that any image that we successfully decode we can use as a cursor? If so, this makes sense; otherwise you need a list. > /** > * Get the cursor for a given frame. > */ >- NS_IMETHOD GetCursor(nsPresContext* aPresContext, >- nsPoint& aPoint, >- PRInt32& aCursor) = 0; >+ NS_IMETHOD GetCursor(nsPoint& aPoint, >+ Cursor& aCursor) = 0; If you're here, you should make this |const nsPoint&|. And yes, I'd prefer if you edited the unrelated changes out of the patch so I know what it is I'm reviewing.

(In reply to comment #67) > Is it true that any image that we successfully decode we can use as a cursor? > If so, this makes sense; otherwise you need a list. Yeah.. that should be true. I certainly intend it to be that way. > If you're here, you should make this |const nsPoint&|. good point. > And yes, I'd prefer if you edited the unrelated changes out of the patch so I > know what it is I'm reviewing. well, I meant to have them reviewed as well and check them in w/ the rest. they're just unrelated in that they are not needed to fix this bug. thinking about it, splitting them out is probably better. I'll attach a new patch tomorrow.

- unrelated changes removed - using const nsPoint& in the GetCursor signature - make sure never to add null requests to nsStyleUserInterface::mCursorArray

Comment on attachment 155390 [details] [diff] [review] windows part oops, this mishandles images w/ 1-bit transparency (.gif)

now supports opaque cursors too, and gifs as cursors. also, checks allocations for failure.

gtk2 patch v1. supports all testcases I could come up with. (alpha-blended cursors are nice! ;) ). unfortunately this version still requires gtk 2.4. note that gtk 2.x x <4, as well as gtk 1.2, only support monochrome cursors...

What do we need to do to allow monochrome cursors for older GTK clients? XBM, of course, is monochrome, but it seems that the current XBM code converts the image to 32-bit... I suppose somebody should look at bug 143046 and apply those concepts to keep XBMs at 2-bit... otherwise we're wasting CPU and memory by converting from 2 to 32 and back to 2-bit. Of course this is a tangental issue and probably should have a separate bug opened.

(In reply to comment #74) > What do we need to do to allow monochrome cursors for older GTK clients? convert the 24-bit color data to 1-bit data... I'll probably do that, but I'm not sure of a good algorithm... maybe: one_bit_data = red > 127 || green > 127 || blue > 127; > I suppose somebody should look at bug 143046 and apply those > concepts to keep XBMs at 2-bit... otherwise we're wasting CPU and memory by > converting from 2 to 32 and back to 2-bit. Of course this is a tangental issue > and probably should have a separate bug opened. ew, then the code here becomes even more complex, to deal with all the various possible depths...

Comment on attachment 155173 [details] [diff] [review] content/layout changes, v2 >Index: content/base/src/nsRuleNode.cpp >@@ -2342,19 +2342,23 @@ nsRuleNode::ComputeUserInterfaceData(nsS >+ ui->mCursorArray.AppendObjects(parentUI->mCursorArray); This isn't quite right. I think you want to clear mCursorArray before doing that (for reasons explained later). >Index: content/html/style/src/nsComputedDOMStyle.cpp >+ for (int i = 0; i < count; i++) { PRInt32 i, please? >Index: content/shared/src/nsStyleStruct.cpp You need to fix the copy constructor to copy the parent's mCursorArray. Otherwise the default "inherit" specified by CSS won't work. This is why rulenode needs to clear first; otherwise in some cases you would end up doubling the length of the list. Please use C++-style initializers for the copy constructor too (I know that's not what it does at the moment; just fix the existing 3 members to use them). I wish we could usefully refcount nsCOMArray instances instead of copying. Really, using nsIMutableArray or nsISupportsArray would only make code worse in 3 places (here, by a tad, and in nsFrame and nsComputedDOMStyle) but would reduce memory usage on some pages (those that use user-interface struct properties other than cursor, though I guess there are precious few of those...). >Index: layout/html/base/src/nsFrame.cpp >+void nsFrame::FillCursorInformationFromStyle(const nsStyleUserInterface* ui, I really wish we had a way to ask the toolkit whether it supports the cursor... Document here that this code assumes that if a toolkit supports cursor images like this at all it will support all image formats Mozilla knows about? And maybe file a bug on a lighweight api to ask the toolkit whether a cursor is supported? >+} >+ >+ Extra newline here. >Index: layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp >+ // XXX set aCursor.mContainer too? Yes. Why not just use your static nsFrame method here (called on the ui struct off the childContext)?

> This isn't quite right. I think you want to clear mCursorArray before doing > that Actually, a simpler approach is to just check whether mCursorArray is empty. If it's not, we've already copied from the parent; if it is, just append the parent array. Just comment that that's what's going on if you do it that way. > I really wish we had a way to ask the toolkit whether it supports the > cursor... Never mind me; we're using the imagelib decoders for all this stuff anyway. Using nsCOMArray should be fine; just add comments in nsStyleStruct.h explaining why the memory overhead will be minimal.

(In reply to comment #76) > This isn't quite right. I think you want to clear mCursorArray before doing > that (for reasons explained later). OK. However, it seems like this is not a correctness problem, only a perf one; since the newly appended entries will be the same ones as the ones already there, and we are only using the first cursor that's loaded, if any.

Did I get it right: this patch currently does not support Windows ANI (animated cursors), and someone will have to file a separate bug for that? [Just curiosity, not implying any criticism.]

That is correct. Animated gifs are not supported either.

Comment on attachment 155483 [details] [diff] [review] content/layout changes, v3 > nsChangeHint nsStyleUserInterface::CalcDifference(const nsStyleUserInterface& aOther) const > { > if (mCursor != aOther.mCursor) > return NS_STYLE_HINT_VISUAL; > >+ // We could do better. But it wouldn't be worth it, URL-specified cursors are >+ // rare. >+ if (mCursorArray.Count() > 0 || aOther.mCursorArray.Count() > 0) >+ return NS_STYLE_HINT_VISUAL; >+ I think both the existing code and the new code you're adding are wrong. How does a repaint cause the cursor to be updated? Doesn't that just happen on mouse move? If a repaint does actually update the cursor, make this code correct (comparing the array), otherwise replace both it and the existing cursor code with a comment. (BTW, if it doesn't update the cursor, probably the best solution is to add a new hint -- we have room for plenty more hints.)

Comment on attachment 155483 [details] [diff] [review] content/layout changes, v3 looks like you are right. I'll add a new style hint (eChangeHint_UpdateCursor)

now with a new style change hint, so that dynamic cursor changes work correctly (this also fixes dynamic changes of non-url cursors). also, triggers start of cursor loads in nsStyleContext::ApplyStyleFixups, so that they are already available when users hover over an element with a custom cursor.

Comment on attachment 155496 [details] [diff] [review] content/layout changes v4 Looks good. r+sr=bzbarsky

Comment on attachment 155401 [details] [diff] [review] windows patch, v2 This doesn't compile: c:\mozilla_source\mozilla\widget\src\windows\nsWindow.h(361) : error C2061: syntax error : identifier 'imgIContainer' + HDC dc = ::GetDC(NULL); ReleaseDC seems to be missing from DataToBitmap. + head.biHeight = h; // Our image is top-down Is this comment relevant?

fixes ere's comments. also, updated to trunk (previous patch doesn't apply cleanly)

Comment on attachment 157887 [details] [diff] [review] windows patch, v3 Code looks fine. Two questions: -- Why aren't we supporting 8bit alpha on Windows? -- I'd be happier if the malloc(w*h) were converted to the correct size. It's not hard...

This patch will only implement the CSS 2.1 version as mentioned in comment 60, correct?

yes, that is correct.

(In reply to comment #88) > -- Why aren't we supporting 8bit alpha on Windows? Hm... testing seems to show that windows does not really support that. But since I can save code by supporting 8bpp in DataToBitmap and getting rid of Conv8to1, which also makes the code support cursors with a width that is not a multiple of 8, I'm doing it. > -- I'd be happier if the malloc(w*h) were converted to the correct size. It's > not hard... Ah yeah, now that I do calculate out_bpr here, it's trivial - out_bpr * h.

> Ah yeah, now that I do calculate out_bpr here, it's trivial - out_bpr * h. although the code is now gone anyway :-)

per discussion w/ pavlov, adds an "readonly attribute nsIProperties properties" attribute on imgIContainer on which the coordinates are stored as wrapped integers.

Comment on attachment 158928 [details] [diff] [review] imagelib part, v2 I suspect that this information should live on the gfxIImageFrames instead of on the container since you could have different data per frame.. for hotspots, that would be a little weird, i guess, but you could certainly have different comments per frame in APNG...

updated per imagelib changes. also, removes unused bpr argument from DataToBitmap, and changes another malloc(w*h) to use the correct dimensions.

pavlov: yes, but I may also have image-global data I want to store. maybe I want to attach EXIF data to an image, or the comment field of an image format.

Comment on attachment 158930 [details] [diff] [review] windows patch, v5 hAlpha uninitialized if format == gfxIFormats::BGR and the malloc fails Win2K and WinXP definitely support 8bit alpha in cursors. The system cursors have it. Why shouldn't it work for us?

ok, I found: http://support.microsoft.com/default.aspx?scid=kb;en-us;318876 "Alpha blended cursors and alpha blended icons are only supported on Microsoft Windows XP." so it seems that windows wants the alpha information to be in the HBITMAP itself. since mozilla stores it elsewhere, I'll have to rewrite the image data, inserting the alpha channel. fun... and it looks like I can't reuse the gtk2 code for this. *sigh* oh, and I probably need OS-detection somewhere in there.

Should the target milestone be moved since it has passed?

how does that matter. but ok, doing it just for you.

(In reply to comment #100) > how does that matter. but ok, doing it just for you. If it doesn't matter, it should just be taken out of bugzilla then.

(In reply to comment #74) > What do we need to do to allow monochrome cursors for older GTK clients? I'm starting to wonder whether this is worth it... I now saw how a monochrome cursor looks (after I implemented this for qt. which, as I then noticed, does not support colored cursors. grrr), I decided that I'll only implement this for gtk 2.4.

this should work w/ gtk2.x x<4 users (in the sense that mozilla will work), although they will not see such cursors.

not going to attach an updated gtk patch just yet, the change is trivial anyway

Comment on attachment 160412 [details] [diff] [review] gtk2 patch, v2 I'd really like it if we could share this code to convert a gfxIImage to a GdkPixbuf. One way we could do this would be to subclass gfxImageFrame for gtk, and add a new interface that has a method something like: GdkPixbuf* ConvertToPixbuf(PRBool aForceAlphaChannel); What do you think?

Comment on attachment 160412 [details] [diff] [review] gtk2 patch, v2 I guess this is ok, see my previous comment about code sharing though.

Hi, cursor from CSS is just perfect for our website. I just tried with smal 32x32 pixel PNG but it doesn't work in Firefox 1.0. What formats are currently supported?

Not supported. The bug is not yet fixed.

I'm not sure whether it's a closely related problem or not, but anyway there's a problem to solve. Windows icons have a MIME type (image/x-icon or, more standard-looking, image/vnd.microsoft.icon -- see http://www.iana.org/assignments/media-types/image/vnd.microsoft.icon for more details). However, there's probably no standard MIME for Windows cursors. IIRC, Mozilla always preferred MIME-based image type guessing -- not extension-based. (Here "extension" is "file name suffix", not "XUL overlay XPI".) So are we going to break this usual preference, or are we going to introduce yet another non-standard MIME type (e.g. image/x-mswindows-cursor), or are there any already existent MIME type for cursors? I have a strong feeling that it would be better to make this guess result extension-based, since that's how the already existent MSIE implementation works, and so we cannot expect most of webservers tuned to support something unique as the MIME type used for cursors; quite the contrary, most possible of all, it's usuallly set to something as mundane as application/octet-stream, or, worse, text/plain. If you feel that the above described problem is actually a separate bug, then please file it as a separate bug in Bugzilla, and add this bug as a blocker of that new bug, and add me to CC list of that new bug. Thank you.

>IIRC, Mozilla always preferred MIME-based image type guessing -- not >extension-based. content-based, actually. (if that doesn't give something useful, it falls back to mimetype based. the extension, if any, is ignored). see also http://www.mozilla.org/docs/web-developer/mimetypes.html#http

(In reply to comment #109) AFAICS Mozilla's image handler recognizes the file type by file content, i.e. as long as it is clear that the file must be an image (for example because it is referred to as a shortcut icon or loaded from an html <img> element) the image will be displayed properly, regardless of the MIME type sent by the server. I think cursors would (or at least should) be handled in the same way.

Ok, so the server-given MIME type is irrelevant to the matter in hand. Fine. Great. Now to some other issue. The above given patches are not implementing ANI animated cursors for Windows. And now we've got a reason not to hurry for it. Microsoft Windows Kernel is vulnerable (and there's no patch except WinXP SP2), see details here: http://www.securityfocus.com/archive/1/385340

However, LoadImage API for Windows also has unpatched vulnerabilities not only for ANI (as described by URL given in my previous comment). The CUR files are also affected: http://www.securityfocus.com/archive/1/385342

So, if the above given patches somehow (directly or, more likely, consequently) use LoadImage() calls on Windows, then additional checks should be added unless we want the whole system compromised by a maliciously composed CUR file. However, I'm not enough skilled to see whether LoadImage() is used. Maybe it's a false alarm after all.

people, we are not using LoadImage. we wouldn't use it for ANI either (at least, not if I am to implement it).

updated per file move + bug 263671

forgot to post this as part of the content/layout patch v5. it was included in the previous versions though, so it was reviewed.

Comment on attachment 169908 [details] [diff] [review] dom patch, v5 (checked in) The cross-platform parts have been checked in (except imagelib). note that there are no visible changes yet; waiting on review for the imagelib patch.

This might be a stupid question, but will the cursor handling routines follow the user's privacy settings regarding images, namely not to load external images in mailnews, not to load images from third party servers, or not to load images at all?

this will do the same as content:url() does now - it checks nsContentUtils::CanLoadImage. this one does a contentpolicy check. looking at the code, this will do the right thing as far as mailnews/remote images are concerned, as well as for disabled images.

The cross-platform change has broken the build under BeOS. It's an error about one of the BeOS files shadowing the new BaseWidget::SetCursor method (I guess we inherit the old SetCursor one but not the new one). BeOS cursors are very limited - pixels are either black, white, or transparent, limited to 16x16 IIRC, and static (I think they chose a "low common denominator" so they could have hardware cursor support on as many machines as possible). What's the best way to get it to build again? Override the method and return NS_ERROR_NOT_IMPLEMENTED, as in the BaseWidget implementation?

>It's an error about >one of the BeOS files shadowing the new BaseWidget::SetCursor method that should be just a warning, not an error... if adding a SetCursor(imgIContainer*) that just returns NS_ERROR_NOT_IMPLEMENTED makes it build again though, then yes, that would be an ok fix...

(In reply to comment #122) > >It's an error about > >one of the BeOS files shadowing the new BaseWidget::SetCursor method > > that should be just a warning, not an error... if adding a > SetCursor(imgIContainer*) that just returns NS_ERROR_NOT_IMPLEMENTED makes it > build again though, then yes, that would be an ok fix... It actually is a warning: the actual error is above that line. But anyway, what harm can it do, that this method is 'hidden'? (What does it mean actually?)

Sorry for this mess of reassigning... I hate these laptop touchpad mice (left mine at home).

Comment on attachment 161501 [details] [diff] [review] imagelib patch, v3 The CID should probably be updated.

with the updated CID, and updated to trunk

this patch depends on bug 276692. re-requesting review since the patch now looks quite a bit different.

due to popular demand, now with alpha support on winxp. the check does not include win2k since the microsoft.com link I gave above claims it does not work there.

Comment on attachment 171829 [details] [diff] [review] windows patch, v6 I didn't have time to really go throught it yet, but I have one request right away: please make the variable names more readable. I can guess that parameter w is the width, but aWidth would be so much better. Or iBpr.. I'd really like aImageBytesPerRow better. I know it's longer, but it's oh so clean. > osversion.dwMajorVersion > 5 || // Newer than Windows Server 2003 Are you sure the next version isn't 5.something? Why are many of the functions static and not members of nsWindow?

(In reply to comment #129) > > osversion.dwMajorVersion > 5 || // Newer than Windows Server 2003 > > Are you sure the next version isn't 5.something? Not actually... I should probably rephrase the comment. > Why are many of the functions static and not members of nsWindow? No good reason I guess... I didn't expect callers outside of nsWindow.cpp, and this way the compiler can maybe generate better code. I'll change it if you want.

now with static member functions and better variable names. I left IsCursorTranslucencySupported as a global static for consistency with IsAlphaTranslucencySupported.

Comment on attachment 172452 [details] [diff] [review] windows patch, v7 Image data is left locked in SetCursor if frame->GetAlphaBytesPerRow(&abpr) fails. + // We will have 32 bpp, so bpr will be 4 * w Make that bpr "bytes per row" and I'm happy.

ok, made those two changes

Having this bug fixed is an important step in the matter of CSS2 compliance (bug 1916), and also MSIE parity (bug 164421) in the hard times of forthcoming MSIE7beta. So I am setting now blocking-? flags for the 1.8beta and aviary1.1 branch -- in hope that Moz1.8 and FF1.1 final releases could support cursor: url(...).

Comment on attachment 175953 [details] [diff] [review] windows patch, v8 r=emaijala

Comment on attachment 175953 [details] [diff] [review] windows patch, v8 > alpharow Make it alphaRow + // Make up an opaque alpha channel + PRUint32 abpr = ((width / 8) + 3) & ~3; + PRUint8* opaque = (PRUint8*)malloc(abpr * height); + if (opaque) { + memset(opaque, 0xff, abpr * height); + + hAlpha = DataToBitmap(opaque, width, height, 1); + free(opaque); + } Factor this into its own static function

When will it be checked in? I am eager to try this improvement, even if in a nightly build!

"soon" :) a recent unrelated checkin made it necessary to fix bug 285048, because otherwise, certain cursors would display very wrong. so, I'm waiting for review on the patch there.

Comment on attachment 171828 [details] [diff] [review] gtk2 patch, v3 Just a couple of small things: >--- widget/src/gtk2/nsWindow.cpp 19 Jan 2005 02:38:58 -0000 1.127 >+++ widget/src/gtk2/nsWindow.cpp 20 Jan 2005 01:49:17 -0000 >@@ -33,12 +33,14 @@ > * and other provisions required by the GPL or the LGPL. If you do not delete > * the provisions above, a recipient may use your version of this file under > * the terms of any one of the MPL, the GPL or the LGPL. > * > * ***** END LICENSE BLOCK ***** */ > >+#include <prlink.h> >+ We generally use "" instead of <> for NSPR includes. > NS_IMETHODIMP >+nsWindow::SetCursor(imgIContainer* aCursor) >+{ >+ if (!sPixbufCursorChecked) { >+ PRLibrary* lib; >+ _gdk_cursor_new_from_pixbuf = (_gdk_cursor_new_from_pixbuf_fn) >+ PR_FindFunctionSymbolAndLibrary("gdk_cursor_new_from_pixbuf", &lib); >+ sPixbufCursorChecked = PR_TRUE; >+ } >+ if (!_gdk_cursor_new_from_pixbuf) >+ return NS_ERROR_NOT_IMPLEMENTED; >+ >+ // if we're not the toplevel window pass up the cursor request to >+ // the toplevel window to handle it. >+ if (!mContainer && mDrawingarea) { >+ GtkWidget *widget = >+ get_gtk_widget_for_gdk_window(mDrawingarea->inner_window); >+ nsWindow *window = get_window_for_gtk_widget(widget); >+ return window->SetCursor(aCursor); >+ } Can you move this toplevel window check up to the beginning of the method? That way we don't recheck sPixbufCursorChecked / _gdk_cursor_new_from_pixbuf. Looks fine otherwise.

transferring r=bryner

Comment on attachment 176749 [details] [diff] [review] gtk2 patch, v4 + if (cursor) { If this fails, shouldn't you unref the pixbuf?

windows patch checked in Checking in widget/src/windows/Makefile.in; /cvsroot/mozilla/widget/src/windows/Makefile.in,v <-- Makefile.in new revision: 3.24; previous revision: 3.23 done Checking in widget/src/windows/nsWindow.cpp; /cvsroot/mozilla/widget/src/windows/nsWindow.cpp,v <-- nsWindow.cpp new revision: 3.544; previous revision: 3.543 done Checking in widget/src/windows/nsWindow.h; /cvsroot/mozilla/widget/src/windows/nsWindow.h,v <-- nsWindow.h new revision: 3.198; previous revision: 3.197 done

roc, thanks for catching that. I actually need to always unref the pixbuf.

bugs filed: Bug 286303 for the CSS 3 syntax (specifying the hotspot in CSS) Bug 286304 for implementing this on mac Bug 286306 for OS/2 Bug 286307 for QNX Bug 286309 for Xlib Bug 286310 for Qt. gtk2: Checking in widget/src/gtk2/Makefile.in; /cvsroot/mozilla/widget/src/gtk2/Makefile.in,v <-- Makefile.in new revision: 1.37; previous revision: 1.36 done Checking in widget/src/gtk2/nsWindow.cpp; /cvsroot/mozilla/widget/src/gtk2/nsWindow.cpp,v <-- nsWindow.cpp new revision: 1.133; previous revision: 1.132 done Checking in widget/src/gtk2/nsWindow.h; /cvsroot/mozilla/widget/src/gtk2/nsWindow.h,v <-- nsWindow.h new revision: 1.52; previous revision: 1.51 done

I'll try to get an XBM patch up this weekend.

this is the fix I had to checkin for the bustage on the egg tinderbox, which is running gtk 2.0.6, which lacks GdkDisplay*.

Getting the following error on Win32:- e:/mozilla/source/mozilla/widget/src/windows/nsWindow.cpp: In function `PRBool I sCursorTranslucencySupported()': e:/mozilla/source/mozilla/widget/src/windows/nsWindow.cpp:181: error: ISO C++ fo rbids declaration of `didCheck' with no type e:/mozilla/source/mozilla/widget/src/windows/nsWindow.cpp:182: error: ISO C++ fo rbids declaration of `isSupported' with no type

Please don't reopen bugs for build bustage unless the checkin is backed out. (The reopening also triggers a cascade of bugmail for the dependencies.)

Sorry, I keep forgetting about that policy change. I'll close this bug and open a new one.

New bug created. Bug #286386 to cover regression from this bug.

So this doesn't actually work for me on GTK2 because I hit this early return in widget/src/gtk2/nsWindow.cpp's SetCursor(imgIContainer*): nsCOMPtr<nsIGdkPixbufImage> pixImg(do_GetInterface(frame)); if (!pixImg) return NS_ERROR_NOT_AVAILABLE; I don't actually see how this could work. gfxImageFrame only allows GetInterface to nsIImage. Or did somebody have a patch in their tree to remove that check in gfxImageFrame.cpp?

Changing the above line: - nsCOMPtr<nsIGdkPixbufImage> pixImg(do_GetInterface(frame)); + nsCOMPtr<nsIGdkPixbufImage> pixImg(do_QueryInterface(nsCOMPtr<nsIImage>(do_GetInterface(frame)))); does fix the problem for me.

(In reply to comment #152) > Or did somebody have a patch in their tree to remove > that check in gfxImageFrame.cpp? yes, I do have that in my tree... and I intended to land it as part of bug 276692, see bug 276692 comment 4 and bug 276692 comment 5; I apparently forgot to diff gfx/shared in that patch :( I'll land that later today.

Hmmm... is Win9x platform supported? Testcase (the 'URL') does nothing in my environment (WinMe, beast-trunk build). Mozilla/5.0 (Windows; U; Win 9x 4.90; ja-JP; rv:1.8b2) Gecko/20050317 Firefox/1.0+ And testcase below (written in Japanese, 3rd <table>, 1st <li> is the testcase of 'cursor:url()') also does nothing, and errors are shown in JavaScript console. http://www5e.biglobe.ne.jp/~access_r/hp/css/css_cursor_001.html > Error: Error in parsing value for property 'cursor'. Declaration dropped. > Source File: http://www5e.biglobe.ne.jp/~access_r/hp/css/css_cursor_001.html > Line: 9 This testcase works with WinIE6. This error message is shown when declaration is written as 'cursor: url()'. Modify this to 'cursor: url(), ...', no errors are shown.

(In reply to comment #155) > Hmmm... is Win9x platform supported? should be, but I have no win9x system to test. > Testcase (the 'URL') does nothing in my environment (WinMe, beast-trunk build). > Mozilla/5.0 (Windows; U; Win 9x 4.90; ja-JP; rv:1.8b2) Gecko/20050317 Firefox/1.0+ :/ guess I'll need to find a win9x system and do some debugging. none of the cursors there work? Can you file a new bug on that? > > Error: Error in parsing value for property 'cursor'. Declaration dropped. That error is correct. You always need one of the cursor keywords. See the grammar at http://www.w3.org/TR/CSS21/ui.html#cursor-props

ah, that the cursors in the testcase field don't work on win9x could be due to: Windows 95/98/Me: The width and height of the cursor must be the values returned by the GetSystemMetrics function for SM_CXCURSOR and SM_CYCURSOR. the testcase there uses cursors of sizes that are, hm, not very likely to correspond to those values ;)

(In reply to comment #156) > (In reply to comment #155) > > Hmmm... is Win9x platform supported? > > should be, but I have no win9x system to test. > > > Testcase (the 'URL') does nothing in my environment (WinMe, beast-trunk build). > > Mozilla/5.0 (Windows; U; Win 9x 4.90; ja-JP; rv:1.8b2) Gecko/20050317 Firefox/1.0+ > > :/ guess I'll need to find a win9x system and do some debugging. none of the > cursors there work? Can you file a new bug on that? Ok. I've filed Bug 286622. > > > Error: Error in parsing value for property 'cursor'. Declaration dropped. > > That error is correct. You always need one of the cursor keywords. See the > grammar at http://www.w3.org/TR/CSS21/ui.html#cursor-props And this is Bug 286624 - for IE-compatible.

One of the features that this fix allows for is specification of custom cursors in usercontent.css for specific link types (e.g. javascript links). Unfortunately one cannot specify file:/// type URIs for cursors due to security restrictions. These restrictions limit the functionality of this fix. Seems to me that there should be some way to allow for file:/// uri cursors if they are specified in usercontent.css rather than by the page content. I realize that this issue is probably well outside the scope of this bug, but I think it is very relevant to it.

Comment 159 should have been a new bug on file, or a reference to an existing bug asking for usercontent.css-specified cursors. /be

I'm sure that the same issue as for file:// cursors in userContent.css exists for -moz-binding, list-style-image, background-image etc., and a fix should address all of those.

(In reply to comment #156) > > > Error: Error in parsing value for property 'cursor'. Declaration dropped. > > That error is correct. You always need one of the cursor keywords. See the > grammar at http://www.w3.org/TR/CSS21/ui.html#cursor-props Because there are so few sites out there that actually document this correctly because IE sucks, I filed some Tech Evangelism bugs for some sites I found while checking this. Do a search for bugs containing "[invalid-css-cursor]" in the Status Whiteboard to find them, and feel free to file bugs for sites that incorrectly document this and add "[invalid-css-cursor]" to the Status Whiteboard.

I have a patch for XBM cursor hotspots. However, it seems that the hotspot does not work in the first place. I tested this functionality with both the CUR code as well as my XBM code and it always sets the hotspot at the top left corner. Should I open a bug for that?

Ignore that last comment, I didn't have a good cursor file.

(In reply to comment #152) > So this doesn't actually work for me on GTK2 I now checked in a fix to make this work.

looks like I forgot to file the beos version of the bug before, here it is: Bug 298184

I can't get this to work in Firefox nightly 20050621 on Windows XP with SP2. I tried the japanese testpage and I tried it on my page: http://jlp.homelinux.org/~jlp/projlab2004/galerija.html (a custom spyglass cursor should show up over images)

The cursor referenced on that page does not exist on the server, so of course it doesn't work.

that page wfm with suite nightly 2005061905. I guess the missing cursor mentioned in comment 168 got added in the meantime...

Yup, it works just fine now. I forgot that i changed the case of the first letter but didn't update the CSS file. I feel quite stupid for spaming because of this. Anyways, thanks to all for making it possible!

(In reply to comment #160) > Comment 159 should have been a new bug on file, or a reference to an existing > bug asking for usercontent.css-specified cursors. See bug 310165.