Closed Bug 840832 Opened 7 years ago Closed 7 years ago

Closing the only visible chat may not select the newly displayed chat.

Categories

(Firefox Graveyard :: SocialAPI, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 21

People

(Reporter: markh, Assigned: markh)

References

Details

Attachments

(1 file, 1 obsolete file)

STR:
* Open 2 chats, then resize the window such that only 1 chat is visible (ie, so the second is "collapsed".
* Close the visible chat.

Expected:
* The chat that was collapsed is made visible and selected.

Actual:
* The chat that was collapsed is visible, but *not* selected.

The problem is that we attempt to locate a new chat to select before we have uncollapsed any other chats.  The fix is to perform the uncollapse before we locate a new chat to select.

FWIW, this was causing some test failures - if the window size happened to be such that only 1 chat was visible on the screen, the attempt by some of the tests to cleanup chats would not be successful - then further tests would fail as they had pre-existing chat windows they did not expect.
This patch arranges so that on chatbox close, the chatbox resize() method is called before attempting to select a new chat.  Also:

* It tweaks the chatbox test to check there are no chats left behind after every sub-test - this change would have made it more obvious what the test failures were - the fact chats were left behind after a sub-test caused a hard-to-diagnose failure (especially as it only happens on some screen-sizes).

* There is a new explicit test for this case, so we don't rely on an accident of screen-size to see this failure case.  The new test explicitly resizes the window such that this condition must be hit.

This bug was first discovered via bug 719318, which caused the default screen-size to be such that the bug was hit.  I've pushed a try run with the patch from that bug, plus this bug, to https://tbpl.mozilla.org/?tree=Try&rev=ffb3e2b9a30d - hopefully it will pass all chat tests in that context.
(In reply to Mark Hammond (:markh) from comment #1)
> This bug was first discovered via bug 719318, which caused the default
> screen-size to be such that the bug was hit.  I've pushed a try run with the
> patch from that bug, plus this bug, to
> https://tbpl.mozilla.org/?tree=Try&rev=ffb3e2b9a30d - hopefully it will pass
> all chat tests in that context.

Looks like the try run failed on the chat test. :(
Blocks: 719318
New try with a new patch at https://tbpl.mozilla.org/?tree=Try&rev=9c5b854b8845 - I'll upload the patch once it passes (or keep going until I get a patch that does pass ;)
I tweaked one additional test which isn't strictly related to this "selectedChat" issue, but was found by the previous try run.  It is a relatively simple improvement, so I've kept it in this patch rather than opening a new bug specifically for that.

A new try run with this patch is at https://tbpl.mozilla.org/?tree=Try&rev=9c5b854b8845 and is looking good so far (but try is along way behind - hopefully the other platforms will also go green while I'm sleeping :)

Asking both Jared and Felipe for review in the hope this can get resolved sooner rather than later - first in gets a door prize*!

[*]<fine-print>Conditions may vary - first in may not actually get a door prize.</fine-print>
Attachment #713239 - Attachment is obsolete: true
Attachment #713299 - Flags: review?(jwein)
Attachment #713299 - Flags: review?(felipc)
Attachment #713299 - Flags: review?(felipc) → review+
Attachment #713299 - Flags: review?(jAwS)
https://hg.mozilla.org/mozilla-central/rev/8533d88c37f1
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
Verified fixed with Firefox 21.0b6.
Status: RESOLVED → VERIFIED
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.