Closed Bug 462245 Opened 13 years ago Closed 13 years ago

Remove override of +[NSGrayFrame drawBevel:...]

Categories

(Core :: Widget: Cocoa, defect)

All
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.1b2

People

(Reporter: mstange, Assigned: mstange)

References

Details

(Keywords: fixed1.9.0.6)

Attachments

(3 files)

Attached patch patch v1Splinter Review
The code in question (from bug 303110) overrides a private method. This can lead to a crash with an awesome stack when Shapeshifter is installed:
http://pastebin.mozilla.org/564163

The "drawing bug" that this override tries to avoid only shows on 10.4 and is really minor (screenshots coming). I think we should just remove it.
Attachment #345378 - Flags: review?(joshmoz)
See if you can spot the difference :-)
Comment on attachment 345378 [details] [diff] [review]
patch v1

Fine with me, those hacks are nasty. I'm sure someone will complain about the little glitch but we should file a bug with Apple.

Please do the following too...
- file a bug with Apple
- put the Apple bug # in this bug
- file a new Mozilla bug on the visual glitch after landing
- link the new Mozilla bug to this bug
Attachment #345378 - Flags: review?(joshmoz) → review+
That bug was 10.4-only. It's fixed in 10.5 so I won't file a new bug.
Attachment #345378 - Flags: superreview?(roc)
Attachment #345378 - Flags: superreview?(roc) → superreview+
Attachment #345378 - Flags: approval1.9.1b2?
Comment on attachment 345378 [details] [diff] [review]
patch v1

a=beltzner, mmm, nasty hacks
Attachment #345378 - Flags: approval1.9.1b2? → approval1.9.1b2+
http://hg.mozilla.org/mozilla-central/rev/3eaa593394b4
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
Markus/Josh/etc.: is this 1.9.0-fodder?  

I don't actually know how common the fun crashes are (I've personally only heard from the one Camino user we saw in #camino that day), but I did notice this bug come up again when Wladimir recently pointed to the list of TomTom's patches: http://www.tomtom.com/page.php?Page=mpl
Thanks a lot for pointing me to this bug - I always had a problem with this particular patch because it was applied without really understanding the implications and because it didn't follow out usual procedure of submitting it for a review here before we apply it to our XULRunner. But looking at the patch here, it does exactly the same thing so apparently it was the right thing to do after all.
Btw, we did release TomTom HOME 2.3 and 2.4 on XULRunner 1.9.0 with this patch applied and I am not aware of any issues. So from what I can tell, it is safe.
Blocks: tomtom
Josh, Markus: Should we take this on 1.9.0?
Flags: wanted1.9.0.x?
I wouldn't mind putting this on 1.9.0.
Comment on attachment 345378 [details] [diff] [review]
patch v1

I also think we should take it.
Attachment #345378 - Flags: approval1.9.0.6?
Flags: wanted1.9.0.x? → wanted1.9.0.x+
Comment on attachment 345378 [details] [diff] [review]
patch v1

Approved for 1.9.0.6, a=dveditz for release-drivers.
Attachment #345378 - Flags: approval1.9.0.6? → approval1.9.0.6+
Checking in widget/src/cocoa/nsCocoaWindow.mm;
/cvsroot/mozilla/widget/src/cocoa/nsCocoaWindow.mm,v  <--  nsCocoaWindow.mm
new revision: 1.150; previous revision: 1.149
done
Keywords: fixed1.9.0.6
Is there any real scenario here to reproduce the crash?
Yes, I think it's as easy as:
1. Use Mac OS 10.4.
2. Install ShapeShifter: http://unsanity.com/haxies/shapeshifter
3. Try to start Firefox.

I haven't tried it myself, though.

This mozillaZine thread shows that the problem is fairly common: http://forums.mozillazine.org/viewtopic.php?f=38&t=671085&st=0&sk=t&sd=a&start=75
You need to log in before you can comment on or make changes to this bug.