786477 - collapsing chatboxes on overflow is wonkey

Status: RESOLVED FIXED

(Firefox Graveyard :: SocialAPI, defect)

Description

[Shane Caraveo (:mixedpuppy)]

after css changes in bug 785593 and bug 784272, collapsing chatboxes during overflow doesn't always work right.

Comment 1

[Shane Caraveo (:mixedpuppy)]

Attachment: overflow width.patch

Comment 2

[:Felipe Gomes (needinfo for replies!)]

is it possible to find an alternative to not have to use this hardcoded value? otherwise every style change we make will end up breaking this again

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Shane, can you use the XUL overflow event? https://developer.mozilla.org/en-US/docs/XUL/Events

Comment 4

[Shane Caraveo (:mixedpuppy)]

(In reply to Jared Wein [:jaws] from comment #3)
> Shane, can you use the XUL overflow event?
> https://developer.mozilla.org/en-US/docs/XUL/Events

we do use overflow.  what doesn't work is using underflow to decide whether to show one of the collapsed chats.  underflow only happens the first underflow, we need to continue checking

Comment 5

[:Felipe Gomes (needinfo for replies!)]

An idea: when we collapse a chat due to overflow, we could store its size as an attribute and then use it here.

Comment 6

[Shane Caraveo (:mixedpuppy)]

(In reply to :Felipe Gomes from comment #5)
> An idea: when we collapse a chat due to overflow, we could store its size as
> an attribute and then use it here.

I was trying to avoid that, but it looks like boxObject.width == 0 when collapsed, so we may have to do that.

Comment 7

[:Felipe Gomes (needinfo for replies!)]

slightly cleaner would be to assume all chatboxes are the same size and store the size in a field in the chatbar rather than on each chatbox

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

How does this patch handle minimized chats? They have a different width (160px + 4px margin start).

Comment 9

[Shane Caraveo (:mixedpuppy)]

Attachment: overflow width.patch

revised patch handles dynamic widths, works well with collapsed chats also.

Comment 10

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 656661 [details] [diff] [review]
overflow width.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 785593, 784272
User impact if declined: bad ux in socialapi chat boxes
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch: none

Comment 11

[Shane Caraveo (:mixedpuppy)]

https://tbpl.mozilla.org/?tree=Try&rev=5a09186a68fc

Comment 12

[:Gavin Sharp [email: [e-mail]]]

I think you're not supposed to use boxObject.width and use getBoundingClientRect().width instead.

Comment 13

[Neil Deakin]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #12)
> I think you're not supposed to use boxObject.width and use
> getBoundingClientRect().width instead.

That's correct. boxObject.x/y/width/height are deprecated. They both should return the same value for width though, although the latter accounts for applied transforms.

Comment 14

[Shane Caraveo (:mixedpuppy)]

Attachment: overflow width.patch

[Approval Request Comment]
fixes poor overflow handling of chat windows

Comment 15

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 657363 [details] [diff] [review]
overflow width.patch

No need to ask for re-review for that kind of tweak.

Comment 16

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b449366e9663

Comment 17

[:Gavin Sharp [email: [e-mail]]]

checkin-needed for aurora, too

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4b9a9033ae42

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b449366e9663

Comment 20

[u279076]

Can I please get a better definition of "wonky" so I can verify this fixed? Thanks

Comment 21

[Shane Caraveo (:mixedpuppy)]

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #20)
> Can I please get a better definition of "wonky" so I can verify this fixed?
> Thanks

IIRC you could reproduce by:

open 3 chat windows then resize the window, smaller then larger.  One of the chat windows will overflow off the edge of the browser window rather than collapsing into the button.