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

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Markus Stange [:mstange]]

Attachment: patch v1

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.

Comment 1

[Markus Stange [:mstange]]

Attachment: screenshot with DrawingBugWorkaround

Comment 2

[Markus Stange [:mstange]]

Attachment: screenshot without DrawingBugWorkaround, i.e. after the patch

See if you can spot the difference :-)

Comment 3

[Josh Aas]

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

Comment 4

[Markus Stange [:mstange]]

That bug was 10.4-only. It's fixed in 10.5 so I won't file a new bug.

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 345378 [details] [diff] [review]
patch v1

a=beltzner, mmm, nasty hacks

Comment 6

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/3eaa593394b4

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

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

Comment 8

[Wladimir Palant]

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.

Comment 9

[Wladimir Palant]

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.

Comment 10

[Samuel Sidler (old account; do not CC)]

Josh, Markus: Should we take this on 1.9.0?

Comment 11

[Josh Aas]

I wouldn't mind putting this on 1.9.0.

Comment 12

[Markus Stange [:mstange]]

Comment on attachment 345378 [details] [diff] [review]
patch v1

I also think we should take it.

Comment 13

[Daniel Veditz [:dveditz]]

Comment on attachment 345378 [details] [diff] [review]
patch v1

Approved for 1.9.0.6, a=dveditz for release-drivers.

Comment 14

[Markus Stange [:mstange]]

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

Comment 15

[Al Billings [:abillings - ex-MoCo]]

Is there any real scenario here to reproduce the crash?

Comment 16

[Markus Stange [:mstange]]

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