collapsing chatboxes on overflow is wonkey

RESOLVED FIXED in Firefox 17

Status

()

Firefox
SocialAPI
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mixedpuppy, Assigned: mixedpuppy)

Tracking

unspecified
Firefox 18
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(firefox17 fixed)

Details

(Whiteboard: [Fx17][qa?])

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
after css changes in bug 785593 and bug 784272, collapsing chatboxes during overflow doesn't always work right.
(Assignee)

Comment 1

5 years ago
Created attachment 656248 [details] [diff] [review]
overflow width.patch
Assignee: nobody → mixedpuppy
Attachment #656248 - Flags: review?(jaws)
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
Shane, can you use the XUL overflow event? https://developer.mozilla.org/en-US/docs/XUL/Events
(Assignee)

Comment 4

5 years ago
(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
An idea: when we collapse a chat due to overflow, we could store its size as an attribute and then use it here.
(Assignee)

Comment 6

5 years ago
(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.
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
How does this patch handle minimized chats? They have a different width (160px + 4px margin start).
(Assignee)

Comment 9

5 years ago
Created attachment 656661 [details] [diff] [review]
overflow width.patch

revised patch handles dynamic widths, works well with collapsed chats also.
Attachment #656248 - Attachment is obsolete: true
Attachment #656248 - Flags: review?(jaws)
Attachment #656661 - Flags: review?(jaws)
Attachment #656661 - Flags: review?(felipc)
Attachment #656661 - Flags: review?(felipc) → review+
(Assignee)

Comment 10

5 years ago
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
Attachment #656661 - Flags: review?(jaws) → approval-mozilla-aurora?
(Assignee)

Comment 11

5 years ago
https://tbpl.mozilla.org/?tree=Try&rev=5a09186a68fc
I think you're not supposed to use boxObject.width and use getBoundingClientRect().width instead.
(In reply to :Gavin Sharp (use gavin@gavinsharp.com 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.
(Assignee)

Comment 14

5 years ago
Created attachment 657363 [details] [diff] [review]
overflow width.patch

[Approval Request Comment]
fixes poor overflow handling of chat windows
Attachment #656661 - Attachment is obsolete: true
Attachment #656661 - Flags: approval-mozilla-aurora?
Attachment #657363 - Flags: review?(gavin.sharp)
Attachment #657363 - Flags: approval-mozilla-aurora?
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
Comment on attachment 657363 [details] [diff] [review]
overflow width.patch

No need to ask for re-review for that kind of tweak.
Attachment #657363 - Flags: review?(gavin.sharp)
Attachment #657363 - Flags: approval-mozilla-aurora?
Attachment #657363 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/integration/mozilla-inbound/rev/b449366e9663
Keywords: checkin-needed
Target Milestone: --- → Firefox 18
checkin-needed for aurora, too
Keywords: checkin-needed
https://hg.mozilla.org/releases/mozilla-aurora/rev/4b9a9033ae42
status-firefox17: --- → fixed
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b449366e9663
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Can I please get a better definition of "wonky" so I can verify this fixed? Thanks
Whiteboard: [Fx17] → [Fx17][qa?]
(Assignee)

Comment 21

5 years ago
(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.
You need to log in before you can comment on or make changes to this bug.