Closed Bug 349618 Opened 16 years ago Closed 16 years ago

[cocoa Firefox] some CSS cursor values fail to display

Categories

(Core :: Widget: Cocoa, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: phiw2, Assigned: jaas)

References

Details

Attachments

(2 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060821 Minefield/3.0a1
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060821 Minefield/3.0a1

Some CSS cursor values: ne-resize, nw-resize, se-resize, sw-resize, help fail to display.
They display the default value for cursor that was used form the point where the mouse is coming from. 
e.g if one was mousing over text, the cursor would remain the 'text' cursor (I beam).

This works correctly in an equivalent non-cocoa Minefield build and in the latest Camino trunk build

Reproducible: Always
Attached file test case
Mouse over the link or the abbreviation, curssor should change as specified
Status: UNCONFIRMED → NEW
Ever confirmed: true
I can confirm this.
I bet the problem is that the cursor images aren't getting packaged right (mozilla/widget/src/cocoa/cursors). Camino probably does it with its project file, but we need the cursors packaged correctly from the cocoa widgets Makefile in order for it to work with XUL apps like Firefox.

Mark - do you think you could take a look at this?
Duplicate of this bug: 375245
Flags: blocking1.9?
Flags: blocking1.9? → blocking1.9+
Attached patch fix v1.0 (obsolete) — Splinter Review
Attachment #260182 - Flags: review?(mark)
Comment on attachment 260182 [details] [diff] [review]
fix v1.0

Benjamin - where do you these cursors should live in a xulrunner world?
Attachment #260182 - Flags: review?(benjamin)
I would prefer these to live in res/cursors, if possible, which would be

directoryservice.get("GreD")
append('res')
append('cursors')
Attached patch fix v1.1 (obsolete) — Splinter Review
Attachment #260182 - Attachment is obsolete: true
Attachment #260323 - Flags: review?(benjamin)
Attachment #260182 - Flags: review?(mark)
Attachment #260182 - Flags: review?(benjamin)
Attachment #260323 - Flags: review?(benjamin) → review+
Comment on attachment 260323 [details] [diff] [review]
fix v1.1

>+  nsresult rv = NS_GetSpecialDirectory(NS_GRE_DIR, getter_AddRefs(resDir));
>+  NS_ASSERTION(NS_SUCCEEDED(rv), "Problem getting path to cursor image file!");

This should probably bail on failure.

>+  NS_ASSERTION(NS_SUCCEEDED(rv), "Problem getting path to cursor image file!");

As should this.

>+  NSString* pathToImage = [NSString stringWithUTF8String:(const char*)resPath.get()];

This should be nil checked, and bail if it is nil (in case something gives it non-UTF8 somehow)

>+  NSImage* cursorImage = [[[NSImage alloc] initWithContentsOfFile:pathToImage] autorelease];

This should also be nil-checked (in case the image is missing).
Attachment #260323 - Attachment is obsolete: true
Attachment #260477 - Flags: review?(stuart.morgan)
Comment on attachment 260477 [details] [diff] [review]
fix v1.2 (bulletproof edition!)

r=smorgan, although the goto is a bit skanky. (I'd almost rather see a local #define'd macro if you really want to avoid the code repitition.)

Should the last two failures have assertions as well, to make it clear why it failed?
Attachment #260477 - Flags: review?(stuart.morgan) → review+
fixed on trunk

(patch will warn on any failure now per comment #11)
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
With the latest Camino Tinderbox trunk build [Version 2007040316 (1.2+)], the testcase fails as it did in Minefield prior to the landing of this patch.

I get a truckload of logging in the console.app log:
2007-04-04 09:49:07.592 Camino[16903] *** -[NSCFDictionary setObject:forKey:]: attempt to insert nil value

But all works correctly with the Minefield build:
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a4pre) Gecko/20070403 Minefield/3.0a4pre ID:2007040315 [cairo]
Camino positively needs its project file kicked to get this working.  I don't know if there's a separate bug tracking that.
(In reply to comment #14)

So, bug 376404 might fix that ?

At the same time, I think there's a good argument to be made that failure to get a cursor should be intelligently handled at higher levels (it's very easy to fix at the place that's failing now, but it may then choke higher up), rather than exploding.
You need to log in before you can comment on or make changes to this bug.