497591 - Selecting "Tab Bar always stays visible" will crash camino when a very small window is present

Status: RESOLVED FIXED

(Camino Graveyard :: Tabbed Browsing, defect)

Description

[russcgaboy]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en; rv:1.9.0.11) Gecko/2009060219 Camino/2.0b3 (like Firefox/3.0.11)
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en; rv:1.9.0.11) Gecko/2009060219 Camino/2.0b3 (like Firefox/3.0.11)

Under some circumstances doing a search on tripadvisor for a hotel for example "scole inn", and then clicking on the search result will crash camino.  The following error will appear in the console: camino[304] Mozilla has caught an Obj-C exception [NSRangeException: *** -[NSCFArray objectAtIndex:]: index (1) beyond bounds (1)]. I encountered this in normal browsing but it's easiest to reproduce a similar or same crash following steps below.

Reproducible: Always

Steps to Reproduce:
1.Start with clean profile following procedure in Camino FAQ or use troubleshoot camino
2.Set preference "Open tabs instead of windows for links that would open new windows"
3.Go to tripadvisor.com and search for "scole inn" and select first search result
4. Click "Window" and note presence of hidden popunder window. The presence of the popunder window seems needed to produce a crash.
5. Go to preferences and select "Tab bar always stays visible".
Actual Results:  
On my system Camino will crash.

Expected Results:  
Tab Bar is displayed. I am not sure how Camino is supposed to handle the hidden window that is already there when you select "always display tab bar", or how Camino is expected to handle a popunder window if the "open tabs instead of windows for links that would open new windows" preference has been selected.

The tripadvisor site sets a cookie call "CommercePopUnder" and the presence of this cookie and value seems to have something to do with the crash. For example, Camino will NOT crash if the cookie is present with the value "SuppressAll".

Comment 1

[Stuart Morgan]

Awesome! With those steps I can now reproduce the crash :)

The stack is:
__raiseError (in CoreFoundation) + 331
objc_exception_throw (in libobjc.A.dylib) + 40
+[NSException raise:format:arguments:] (in CoreFoundation) + 155
+[NSException raise:format:] (in CoreFoundation) + 58
_NSArrayRaiseBoundException (in Foundation) + 127
-[NSCFArray objectAtIndex:] (in Foundation) + 72
-[NSTabView tabViewItemAtIndex:] (in AppKit) + 44
-[BrowserTabBarView drawRect:] (in Camino) (BrowserTabBarView.mm:189)
-[NSView _drawRect:clip:] (in AppKit) + 3853
...

Which means somehow mLeftMostVisibleTabIndex is wrong. If nothing else we can add a guard there, but I want to dig in and understand how it got to be wrong in the first place.

Thanks for narrowing this down to a set of "clean room" steps!

Comment 2

[Stuart Morgan]

So step 2 turns out not to be necessary. The problem is that the popup window they open is smaller than we normally allow windows to be (which is probably legitimate for popup windows in at least some cases)--so smaller than it's smaller than our normal minimum tab width--and the tab layout code didn't anticipate that possibility. We never caught it in testing the tab stuff because we were resizing the windows manually.

Comment 3

[Stuart Morgan]

Attachment: fix

Comment 4

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

Stuart, can this happen in 1.6.x, too, or is this due to dragging-related changes?

(In reply to comment #2)
> So step 2 turns out not to be necessary. The problem is that the popup window
> they open is smaller than we normally allow windows to be (which is probably
> legitimate for popup windows in at least some cases)--so smaller than it's

I remember commenting on this pop-up when trying to reproduce this bug after seeing the Breakpad reports; I think we really should force pop-ups to be at least a (smaller) useful minimum size and entirely onscreen, if possible.

Comment 5

[Stuart Morgan]

We should land this for 1.6 as well (have we already built the final 1.6.8?)

I was surprised we didn't force popups to stay on the screen; we should definitely file a bug for that.

Comment 6

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

(In reply to comment #5)
> We should land this for 1.6 as well (have we already built the final 1.6.8?)

No, so please cross-land (or spin a branch patch and land) if you get sr before Monday.  If you don't have sr by Monday, please attack pink; assuming Gecko has not experienced additional delays by the time I get back, I will be building 1.6.8 Monday afternoon/evening.

Comment 7

[Chris Lawson (gone)]

(In reply to comment #5)

> I was surprised we didn't force popups to stay on the screen; we should
> definitely file a bug for that.

Filed as bug 497890 at Smokey's request.

Comment 8

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 382738 [details] [diff] [review]
fix

sr=pink

Comment 9

[Stuart Morgan]

Landed on CVS trunk and MOZILLA_1_8_BRANCH.