Closed Bug 1112525 Opened 5 years ago Closed 5 years ago

UiTour: When someone connects to a Hello conversation, infoPanels are incorrectly sized

Categories

(Firefox :: Tours, defect)

36 Branch
All
macOS
defect
Not set
Points:
3

Tracking

()

RESOLVED FIXED
Firefox 37
Iteration:
37.2
Tracking Status
firefox35 --- fixed
firefox36 --- fixed
firefox37 --- fixed

People

(Reporter: agibson, Assigned: MattN)

References

Details

Attachments

(4 files, 1 obsolete file)

When someone connects to a Hello conversation and an infoPanel is displayed, the door-hanger dimensions gets altered and are incorrect (see attachment). This sizing is then displayed wrongly for the duration that Firefox is open, and only corrects itself on restart.

I think this *might* not be a blocker, since once a conversation connects afaik we are hiding info panels and just displaying a congratulations message in in the web page. But it should still get fixed.
Attached image hello-disconnect.png
Looks like the same thing can happen when a room disconnects. In this case, some kind of connection error occurred when I tried to connect - and the info panels broke again. Perhaps this should be considered a blocker?
I'm having some trouble reproducing this again - but will continue to test & try to figure out the circumstances that caused this.
Status: NEW → UNCONFIRMED
Ever confirmed: false
I managed to recreate this again today and get a screen-recording of what happens. This seems to happen for me not every time, but sporadically. Sometimes it happens several seconds after the conversation connects.

For the FTE, our code will hide info panels when we get a 'Loop:IncomingConversation' event as this is required as part of the final step. However if this call to hide is not in the code, this bug seems to manifest.
As per our discussion yesterday, I just tested using gfxCardStatus (https://gfx.io/) with dynamic switching enabled and when the bug occurs, there is an "i" in the finder menu.
User agent is: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:37.0) Gecko/20100101 Firefox/37.0"

I'm running a MacBook Pro with retina display. No external monitors attached
(In reply to Alex Gibson [:agibson] from comment #4)
> As per our discussion yesterday, I just tested using gfxCardStatus
> (https://gfx.io/) with dynamic switching enabled and when the bug occurs,
> there is an "i" in the finder menu.

Does it switch when the bug happens? Can you watch the icon closely when the problem happens to see if it flickers to discrete?
(In reply to Matthew N. [:MattN] from comment #6)
> (In reply to Alex Gibson [:agibson] from comment #4)
> > As per our discussion yesterday, I just tested using gfxCardStatus
> > (https://gfx.io/) with dynamic switching enabled and when the bug occurs,
> > there is an "i" in the finder menu.
> 
> Does it switch when the bug happens? Can you watch the icon closely when the
> problem happens to see if it flickers to discrete?

I didn't see a flicker, but interestingly if I change it to "Discrete only" once the conversation has connected, then the bug occurs as soon as the GPU changes.
Also it's worth noting that sometimes it takes up to 10 seconds for the bug to kick in (without trying to manually forcing it like it Comment 7) once the the conversation connects. The time seems a bit random, so maybe it is triggered by something to do with the GPU.
I can't reproduce the panel layout breaking but I did find a case where the panel position will be wrong and I'm wondering if you just also see the panel layout break in this case:
* Open a room
* Mozilla.UITour.showInfo("loop-selectedRoomButtons", "test", "foo")
* Click the red "Hang up" button to take you to the happy/sad faces.  Note that nobody else should be present.
* Move the window or switch GPU (anything that causes layout to re-calculate). There is no notification sent about this case. I didn't realize you could get to the survey without actually making a call. It seems like a bug in Loop which I'm told is filed.

For me the panel simply moves to the left of the browser window. I'm wondering if you see the broken layout of it too with these STR.
Flags: needinfo?(agibson)
(In reply to Matthew N. [:MattN] from comment #9)
> I can't reproduce the panel layout breaking but I did find a case where the
> panel position will be wrong and I'm wondering if you just also see the
> panel layout break in this case:
> * Open a room
> * Mozilla.UITour.showInfo("loop-selectedRoomButtons", "test", "foo")
> * Click the red "Hang up" button to take you to the happy/sad faces.  Note
> that nobody else should be present.
> * Move the window or switch GPU (anything that causes layout to
> re-calculate). There is no notification sent about this case. I didn't
> realize you could get to the survey without actually making a call. It seems
> like a bug in Loop which I'm told is filed.
> 
> For me the panel simply moves to the left of the browser window. I'm
> wondering if you see the broken layout of it too with these STR.

Yes I can reproduce this and I see the broken panel layout here too. The panel moves off to the left, but then the panel dimensions break too, and then also remain broken until I restart Firefox.
Flags: needinfo?(agibson)
I just managed to hit this bug when joining a room in the regular FTE flow - when a connection was made, the infoPanel didn't hide quite in time and the layout broke :-(
Confirmed per comment #10.
Status: UNCONFIRMED → NEW
Ever confirmed: true
I'm investigating a fix now.
Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED
Iteration: --- → 37.2
Points: --- → 3
Flags: qe-verify-
Flags: firefox-backlog+
Hardware: x86 → All
Note that the Loop bug that would fix comment 9 is bug 1109849.
/r/1609 - Bug 1112525 - UITour: Change the loop-selectedRoomButtons target to anchor from the chatbox's browser so info panels behave when the buttons disappear.

Pull down this commit:

hg pull review -r 4ca3f64d88f254d9a3e3ed48a307346ea506b434
I can no longer trigger this bug with the try build linked in Bug 1113574 Comment 12 \o/

I tested by:

- Joining a room with a door-hanger showing
- Clicking the red "Hang up" button when a door-hanger is showing
- Physically switching GPU to "Discrete only" when the door-hanger is showing

None of these cases seem to exhibit the problem any longer
(In reply to Matthew N. [:MattN] from comment #16)
> /r/1609 - Bug 1112525 - UITour: Change the loop-selectedRoomButtons target
> to anchor from the chatbox's browser so info panels behave when the buttons
> disappear.

Patch looks fine, but do you understand how this was causing the sizing issue in attachment 8537734 [details]?

Is the infoPanelPosition change the key? Sounds a lot like what I ran into in bug 1101669 comment 8, but I think that only manifested as a positioning bug, not a sizing bug. I'd have expected the same for anchoring to elements that disappear.
Flags: needinfo?(MattN+bmo)
(In reply to Justin Dolske [:Dolske] from comment #18)
> (In reply to Matthew N. [:MattN] from comment #16)
> > /r/1609 - Bug 1112525 - UITour: Change the loop-selectedRoomButtons target
> > to anchor from the chatbox's browser so info panels behave when the buttons
> > disappear.
> 
> Patch looks fine, but do you understand how this was causing the sizing
> issue in attachment 8537734 [details]?
>
> Is the infoPanelPosition change the key? Sounds a lot like what I ran into
> in bug 1101669 comment 8, but I think that only manifested as a positioning
> bug, not a sizing bug. I'd have expected the same for anchoring to elements
> that disappear.

There is an underlying issue in <panel>s that needs to be investigated separately but the problem is when the anchor of the popup disappears. The position was just needed because I don't want to anchor on the center of the new anchor. The anchor change itself was the solution. Bug 1109868 would solve this properly and I hope we can get it prioritized so we don't have to continually workaround it in UITour like we have been.

The panel layout issue isn't reproducible for me so it may depend on some other factors such as graphics drivers. I only get the bad positioning so we need a patch like this to deal with that regardless.
Flags: needinfo?(MattN+bmo)
Comment on attachment 8539544 [details]
MozReview Request: bz://1112525/MattN

Ok, Matt clarified with me. In theory the anchor shouldn't have any effect on the panel's size -- only position -- but seems there is some panel wonkyness that makes it break that way, so that's why this patch seems to fix it.
Attachment #8539544 - Flags: review?(dolske) → review+
https://hg.mozilla.org/mozilla-central/rev/f2b28a37a4c0
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 37
Comment on attachment 8539544 [details]
MozReview Request: bz://1112525/MattN

Approval Request Comment
[Feature/regressing bug #]: Hello first time experience (FTE/FTU)
[User impact if declined]: When the email/copy Hello buttons have an annotation and then the buttons go away e.g. if someone joins the room, the tour info panel position will be wrong and in some cases the layout will be wrong until the browser restarts.
[Describe test coverage new/current, TBPL]: Manual testing and QE verification of the tour later
[Risks and why]: Low risk since it's mostly changing the anchor of the info panel to something more stable.
[String/UUID change made/needed]: None
Attachment #8539544 - Flags: approval-mozilla-beta?
Attachment #8539544 - Flags: approval-mozilla-aurora?
Attachment #8539544 - Flags: approval-mozilla-beta?
Attachment #8539544 - Flags: approval-mozilla-beta+
Attachment #8539544 - Flags: approval-mozilla-aurora?
Attachment #8539544 - Flags: approval-mozilla-aurora+
Verified on nightly 37 and beta 36.
Attachment #8539544 - Attachment is obsolete: true
Attachment #8618933 - Flags: review+
You need to log in before you can comment on or make changes to this bug.