Mozilla (Mac version only apparently) crashes when displaying page with :hover :overflow and scrolling

RESOLVED FIXED

Status

Core Graveyard
GFX: Mac
--
critical
RESOLVED FIXED
14 years ago
9 years ago

People

(Reporter: Thilo Planz, Assigned: Jerry Talkington)

Tracking

({crash, fixed-aviary1.0, fixed1.7})

Trunk
PowerPC
Mac OS X
crash, fixed-aviary1.0, fixed1.7

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [have patch] [fixed on trunk]. needed-aviary1.0, ready to land, URL)

Attachments

(3 attachments)

(Reporter)

Description

14 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.6b) Gecko/20031202
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.6b) Gecko/20031202

The attached page uses a div that is hidden unless hovered over.
This div contains a very wide element, that is given scrollbars using overflow:auto

Reproducible: Always

Steps to Reproduce:
1.
2.
3.
(Reporter)

Comment 1

14 years ago
The attached page contains a DIV that is hidden unless hovered upon.
This DIV contains a second DIV with overflow:auto (to give it scrollbars).
The scrolling DIV contains a very wide DIV with some text inside.

At first, the hidden DIV is hidden.
When hovered upon, it is displayed and Mozilla crashes.

I tried Mozilla and Firebird (both on Mac OS X 10.3.1) and both crash.
It seems to work on other platforms and also in Safari.

Is this 10.3-only?  Or also happening on 10.2?

Updated

14 years ago
Severity: normal → critical
Keywords: crash

Comment 3

14 years ago
Confirmed with Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.5)
Gecko/20031007 under Mac OS 10.2.8.

Works with Camino Build ID: 2003120109.

Comment 4

14 years ago
Confirming with 2003120305 on Mac OS 10.1.5. Crash log is coming...
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 5

14 years ago
Created attachment 136734 [details]
crash log

Hmm. I have no idea what component this belongs to...
Component: Browser-General → GFX: Mac
(Assignee)

Comment 6

14 years ago
Actually, the scrollbars aren't necessary to make it crash, nor is the
super wide div.  It's possible to cause the crash with just a div tha
t is hidden until another element is hovered, if the hidden div has
overflow turned on.

Interestingly, Camino displays the text as expected, showing the block when
the visible text is hovered, and continuing to display it if the cursor
passes over newly visible text.  With Firebird (all platforms,) the block
dissapears when the cursor leaves the always visible text, and the side
effect on the Mac platform is a crash.

Let's run this through a debugger:

(gdb) run
Starting program:
/Volumes/Vol2/jerry/src/firebird/Firebird-2003-12-30-02/dist/Mozilla
Firebird.app/Contents/MacOS/MozillaFirebird-bin -P Testing
Reading symbols for shared libraries ..+ done
Reading symbols for shared libraries ...+ done
Reading symbols for shared libraries .....+ done
Reading symbols for shared libraries .....+ done
Reading symbols for shared libraries .....+ done
Reading symbols for shared libraries .....+ done

Program received signal EXC_BAD_ACCESS, Could not access memory.
0x732f4c68 in ?? ()

(gdb) bt
#0  0x732f4c68 in ?? ()
Cannot access memory at address 0x732f4c68
Cannot access memory at address 0x732f4c68
#1  0x0020eeb8 in nsMacEventHandler::HandleMouseMoveEvent(EventRecord&)
(this=0x1a5588d0, aOSEvent=@0xbfffefc0) at nsMacEventHandler.cpp:1780
#2  0x001f4438 in nsMacWindow::DispatchEvent(void*, int*) (this=0x1b15a240,
anEvent=0xbfffed30, _retval=0xbfffeeb0) at
/usr/include/gcc/darwin/3.3/c++/memory:282
#3  0x002082a4 in nsMacMessagePump::DispatchOSEventToRaptor(EventRecord&,
OpaqueWindowPtr*) (this=0x1b15a240, anEvent=@0xbfffefc0, aWindow=0x1a559cc0) at
../../../dist/include/xpcom/nsCOMPtr.h:669
#4  0x00208020 in nsMacMessagePump::DoMouseMove(EventRecord&) (this=0x13f9e0c0,
anEvent=@0xbfffefc0) at nsMacMessagePump.cpp:852
#5  0x00207560 in nsMacMessagePump::DispatchEvent(int, EventRecord*)
(this=0x13f9e0c0, aRealEvent=-1073746640, anEvent=0xbfffefc0) at
nsMacMessagePump.cpp:476
#6  0x00207304 in nsMacMessagePump::DoMessagePump() (this=0x13f9e0c0) at
nsMacMessagePump.cpp:312
#7  0x001ed60c in nsAppShell::Run() (this=0x1442e900) at
/usr/include/gcc/darwin/3.3/c++/memory:282
#8  0x0082f088 in main1(int, char**, nsISupports*, nsXREAppData const&) (argc=3,
argv=0xbffff448, nativeApp=0x0, aAppData=@0xbffff350) at
../../dist/include/xpcom/nsCOMPtr.h:669
#9  0x0082f7f4 in xre_main(int, char**, nsXREAppData const&) (argc=3,
argv=0xbffff448, aAppData=@0xbffff350) at nsAppRunner.cpp:1725
#10 0x00004c24 in main (argc=3, argv=0xbffff448) at nsBrowserApp.cpp:51

(gdb) f 1
#1  0x0020eeb8 in nsMacEventHandler::HandleMouseMoveEvent(EventRecord&)
(this=0x1a5588d0, aOSEvent=@0xbfffefc0) at nsMacEventHandler.cpp:1780
1780                                    retVal |=
widgetPointed->DispatchMouseEvent(mouseEvent);
(gdb) p widgetPointed->DispatchMouseEvent
$1 = &nsWindow::DispatchMouseEvent(nsMouseEvent&)

That's ok.  Let's take a closer look at widgetPointed:
(gdb) p *widgetPointed
$2 = {
  <nsBaseWidget> = {
    <nsIWidget> = {
      <nsISupports> = {
        _vptr$nsISupports = 0x357b0x47726170
          
      }, <No data fields>}, 
    members of nsBaseWidget: 
    mRefCnt = {
      mValue = 0
    }, 
    mClientData = 0x0, 
    mEventCallback = 0x36dd44 <HandleEvent(nsGUIEvent*) at nsView.cpp:68>, 
    mContext = 0x0, 
    mAppShell = {
      <nsCOMPtr_base> = {
        mRawPtr = 0x0
      }, <No data fields>}, 
    mToolkit = 0x0, 
    mMouseListener = 0x0, 
    mEventListener = 0x0, 
    mMenuListener = 0x0, 
    mBackground = 4290822336, 
    mForeground = 4278190080, 
    mCursor = eCursor_standard, 
    mWindowType = eWindowType_child, 
    mBorderStyle = eBorderStyle_none, 
    mIsShiftDown = 0 '\0', 
    mIsControlDown = 0 '\0', 
    mIsAltDown = 0 '\0', 
    mIsDestroying = 0 '\0', 
    mOnDestroyCalled = 0 '\0', 
    mBounds = {
      x = 0, 
      y = 0, 
      width = 1026, 
      height = 180
    }, 
    mOriginalBounds = 0x0, 
    mZIndex = 0, 
    mSizeMode = nsSizeMode_Normal, 
    mChildren = {
      <nsCOMPtr_base> = {
        mRawPtr = 0x1b1595f0
      }, <No data fields>}
  }, 
  <nsDeleteObserved> = {
    _vptr$nsDeleteObserved = 0x9f9eb80x2170f0
<nsDeleteObserved::~nsDeleteObserved() at nsDeleteObserver.cpp:56>
      , 
    mDeleteObserverArray = 0x1b21c690, 
    mObject = 0x1b15a240
  }, 
  <nsIKBStateControl> = {
    <nsISupports> = {
      _vptr$nsISupports = 0x9f9a680x210480 <non-virtual thunk to
nsWindow::QueryInterface(nsID const&, void**) at
../../../dist/include/xpcom/nsID.h:74>
        
    }, <No data fields>}, 
  <nsIPluginWidget> = {
    <nsISupports> = {
      _vptr$nsISupports = 0x9f9a800x210478 <non-virtual thunk to
nsWindow::QueryInterface(nsID const&, void**) at
../../../dist/include/xpcom/nsID.h:74>
        
    }, <No data fields>}, 
  members of nsWindow: 
  mParent = 0x0, 
  mIsTopWidgetWindow = 0 '\0', 
  mResizingChildren = 0 '\0', 
  mSaveVisible = 0 '\0', 
  mVisible = 1 '\001', 
  mEnabled = 1 '\001', 
  mPreferredWidth = 0, 
  mPreferredHeight = 0, 
  mFontMetrics = 0x0, 
  mMenuBar = 0x0, 
  mWindowRegion = 0x0, 
  mVisRegion = 0x0, 
  mWindowPtr = 0x1a559cc0, 
  mDestroyCalled = 1 '\001', 
  mDestructorCalled = 1 '\001', 
  mAcceptFocusOnClick = 1 '\001', 
  mDrawing = 0 '\0', 
  mTempRenderingContextMadeHere = 0 '\0', 
  mTempRenderingContext = 0x0, 
  mPluginPort = 0x0
}


Ooh, mDestroyCalled and mDestructorCalled are positive.  This widget is
getting destroyed, just like the other platforms, but we are still
dispatching a mouse event to it.  Better add a reference to the widget
until we are done with it.

I'll attach a patch to fix the crash, but there is still a problem with
the layout, since the box shouldn't be dissapearing in the first place...
(Assignee)

Comment 7

14 years ago
Created attachment 138176 [details] [diff] [review]
Perform a NS_IF_ADDREF on the widget before sending the mouse event, so it doesn't get destroyed out from under us.

Comment 8

14 years ago
Setting patch keyword and going to have a go applying this patch to my Camino build.
Whiteboard: patch
-> jerry who created this patch

you should probably request review for this patch, try pinkerton or sfraser or
sdagley
Assignee: general → jtalkington
(Assignee)

Updated

14 years ago
Attachment #138176 - Flags: review?(pinkerton)
+		if(widgetPointed) {
+			NS_IF_ADDREF(widgetPointed);
+		}

NS_IF_ADDREF does the if check for you. That's it's sole purpose, otherwise
you'd just use NS_ADDREF. To be more correct, you should probably use a nsCOMPtr
here.

  nsCOMPtr<nsIWidget> kungFuDeathGrip(widgetPointed)

and that'll do the right thing in the event of early unwanted returns.
Attachment #138176 - Flags: review?(pinkerton) → review-

Comment 11

13 years ago
Created attachment 154399 [details] [diff] [review]
Add a "nsCOMPtr kungFuDeathGrip" before handling mouse event.

I don't actually have a firm grasp of what I'm doing here, but I've read up a
little on "nsCOMPtr"s and done my best to reason out what people mean by
"kungFuDeathGrip", so at least I'm not _surprised_ that this patch works.  (I'm
not entirely convinced that this is the best way to write this block of code,
since it feels like the nsCOMPtr would be used directly if this were done
properly.  But I don't know nearly enough or have anywhere near enough time to
do more than implement Mike Pinkerton's suggestion, which was my approach
here.)

I have made no attempt to figure out why the newly visible block disappears
when you try to move your mouse over it; that really ought to get fixed at some
point, too.  (Does it have its own bug?)  At any rate, if this patch isn't good
enough to be incorporated into the browser, I very much hope that someone who
knows more than I do will write one soon.

Comment 12

13 years ago
For the record, the URL listed above no longer appears to be valid.  I've
created a page that suffers from this bug, at

http://home.uchicago.edu/~sbjensen/cssbug.html

(That page crashes my 0.9.1 browser, but not a custom build with my patch above
applied.)
*** Bug 252176 has been marked as a duplicate of this bug. ***
Attachment #154399 - Flags: superreview+
Attachment #154399 - Flags: review?(pinkerton)
Comment on attachment 154399 [details] [diff] [review]
Add a "nsCOMPtr kungFuDeathGrip" before handling mouse event.

r=pink
Attachment #154399 - Flags: review?(pinkerton) → review+
would like to get a= on this for the 1.7 branch.
Flags: blocking1.7.3?
where is the equivalent code in cocoa widget?
patch landed on trunk. leaving open for branch consideration and cocoa widget.

Comment 18

13 years ago
Should we expect to see this patch showing up in the AVIARY branch any time
soon?  (I don't know the procedure for requesting such things, if a specific
request has to be made.)
Flags: blocking-aviary1.0?
Whiteboard: patch → [have patch] [fixed on trunk]
Attachment #154399 - Flags: approval1.7.3?
Attachment #154399 - Flags: approval-aviary?
Whiteboard: [have patch] [fixed on trunk] → [have patch] [fixed on trunk]. needed-aviary1.0

Comment 19

13 years ago
Comment on attachment 154399 [details] [diff] [review]
Add a "nsCOMPtr kungFuDeathGrip" before handling mouse event.

a=asa for branches checkin
Attachment #154399 - Flags: approval1.7.x?
Attachment #154399 - Flags: approval1.7.x+
Attachment #154399 - Flags: approval-aviary?
Attachment #154399 - Flags: approval-aviary+
Whiteboard: [have patch] [fixed on trunk]. needed-aviary1.0 → [have patch] [fixed on trunk]. needed-aviary1.0, ready to land
Checked in to aviary and 1.7 branches.

The testcase works properly in Camino, so I don't think there is anything to do
there.  Marking FIXED for now, but if anyone finds that Camino does need a
similar patch, then please reopen this bug.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Keywords: fixed-aviary1.0, fixed1.7
Resolution: --- → FIXED
(In reply to comment #20)
> Checked in to aviary and 1.7 branches.

Thanks, clearing blocking? flags.
Flags: blocking1.7.x?
Flags: blocking-aviary1.0?
adding to Camino082 tracking list
Blocks: 261393
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.