Closed
Bug 1254104
Opened 8 years ago
Closed 8 years ago
Sharing notifications are not displayed on the Conversation window toolbar in e10s mode
Categories
(Hello (Loop) :: Client, defect, P1)
Hello (Loop)
Client
Tracking
(e10s+, firefox46 affected, firefox47 verified, firefox48 verified)
People
(Reporter: standard8, Assigned: mikedeboer)
References
Details
(Whiteboard: [btpp-fix-now])
Attachments
(2 files)
3.83 KB,
patch
|
evilpie
:
review+
ritu
:
approval-mozilla-aurora+
lizzard
:
approval-mozilla-beta-
|
Details | Diff | Splinter Review |
31.48 KB,
patch
|
mossop
:
review+
ritu
:
approval-mozilla-aurora+
lizzard
:
approval-mozilla-beta-
|
Details | Diff | Splinter Review |
I just noticed a regression in e10s mode - we're not displaying the sharing (mic/camera nor screen) notifications in the conversation window toolbar. They are fine in non-e10s mode. If I go to the global toolbar and select the icon, then "Control Sharing", the pop-ups and icons appear fine. Side note: Although we want to change these at some stage (bug 1216679), we haven't decided exactly on how yet, therefore these should be present until we have a good resolution for that bug. I'm also cautious, as this means something isn't working as it should be in e10s mode. Florian, any ideas on where we should start looking? or for things for e10s that might have affected this?
Flags: needinfo?(florian)
Updated•8 years ago
|
Rank: 16
Whiteboard: [btpp-fix-now]
Assignee | ||
Comment 1•8 years ago
|
||
I'll just try to fix this.
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Iteration: --- → 48.1 - Mar 21
Points: --- → 5
Flags: needinfo?(florian)
Assignee | ||
Comment 2•8 years ago
|
||
This issue was introduced by bug 896748, in other words: a regression that we know about since Loop started running in e10s mode.
Assignee | ||
Updated•8 years ago
|
Points: 5 → 2
Assignee | ||
Comment 3•8 years ago
|
||
Hi Tom, can you review this for me?
Attachment #8730827 -
Flags: review?(evilpies)
Assignee | ||
Updated•8 years ago
|
Assignee | ||
Comment 4•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4614ec5163ba
Updated•8 years ago
|
Comment 5•8 years ago
|
||
Comment on attachment 8730827 [details] [diff] [review] Patch v1: use docShellIsActive instead of introducing logic that relies on tabbrowser Review of attachment 8730827 [details] [diff] [review]: ----------------------------------------------------------------- Makes sense.
Attachment #8730827 -
Flags: review?(evilpies) → review+
Assignee | ||
Comment 6•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=866ab378afb2
Assignee | ||
Comment 7•8 years ago
|
||
Attachment #8731201 -
Flags: review?(dtownsend)
Assignee | ||
Comment 8•8 years ago
|
||
This test fix could be the fix for bug 1233587 as well, if it still occurs that is.
Assignee | ||
Comment 9•8 years ago
|
||
Try is looking nice and green.
Updated•8 years ago
|
Attachment #8731201 -
Flags: review?(dtownsend) → review+
Assignee | ||
Comment 10•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/30e050c04c4e32e77a7fa4afeb05c8c9f1093c30 Bug 1254104: use docShellIsActive instead of introducing logic that relies on tabbrowser implementation specifics. r=evilpies https://hg.mozilla.org/integration/fx-team/rev/4f18cd7a0bc81ce85d0668c86e66156455d9cb84 Bug 1254104: fix timing issues in browser_bug553455.js, which started perma-failing after fixing docShellIsActive getters in PopupNotifications.jsm. r=Mossop
Assignee | ||
Comment 11•8 years ago
|
||
Thanks for the reviews, Tom & Dave!
Comment 12•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/30e050c04c4e https://hg.mozilla.org/mozilla-central/rev/4f18cd7a0bc8
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Assignee | ||
Comment 13•8 years ago
|
||
Comment on attachment 8730827 [details] [diff] [review] Patch v1: use docShellIsActive instead of introducing logic that relies on tabbrowser Approval Request Comment [Feature/regressing bug #]: bug 896748, loop-e10s [User impact if declined]: The user won't see the camera/ mic and browser sharing icons in the titlebar of chat windows in e10s mode without this patch applied. Thus users won't be able to access the device permission doorhangers. [Describe test coverage new/current, TreeHerder]: landed on m-c, tests pass provided that Patch 2 is also present. [Risks and why]: minor, since test coverage is pretty good here. [String/UUID change made/needed]: n/a.
Attachment #8730827 -
Flags: approval-mozilla-beta?
Attachment #8730827 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 14•8 years ago
|
||
Comment on attachment 8731201 [details] [diff] [review] Patch 2: fix timing issues in browser_bug553455.js See comment 13 for approval request comment. Thank you!
Attachment #8731201 -
Flags: approval-mozilla-beta?
Attachment #8731201 -
Flags: approval-mozilla-aurora?
Comment 15•8 years ago
|
||
Comment on attachment 8730827 [details] [diff] [review] Patch v1: use docShellIsActive instead of introducing logic that relies on tabbrowser The e10s experiment isn't going to last much longer on 46 and we aren't shipping with e10s for 46. I don't think it's worth the risk to uplift this to beta 4.
Attachment #8730827 -
Flags: approval-mozilla-beta? → approval-mozilla-beta-
Updated•8 years ago
|
Attachment #8731201 -
Flags: approval-mozilla-beta? → approval-mozilla-beta-
Comment on attachment 8730827 [details] [diff] [review] Patch v1: use docShellIsActive instead of introducing logic that relies on tabbrowser Fix for Hello + e10s. Aurora47+
Attachment #8730827 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8731201 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: qe-verify+
Comment 17•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/774b367ca02d https://hg.mozilla.org/releases/mozilla-aurora/rev/1cdcf38a2a57
Comment 18•8 years ago
|
||
Unable to reproduce this issue with Nightly from 2016-03-07. Confirming that all sharing notifications are displayed with 47 beta (e10s on/off), across platforms [1]. [1] Windows 10 64-bit, Ubuntu 14.04 32-bit, Mac OS X 10.10.5
Comment 19•8 years ago
|
||
I wasn't able to reproduce the issue on Nightly 2016-03-07. I can confirm that the sharing notifications are correctly displayed with Firefox 48 Beta 9, e10s on/off (buildID: 20160718142219), on Windows 7 64bit, Ubuntu 14.04 64bit and Mac OSX 10.9.5.
Comment 20•7 years ago
|
||
Backed out from beta bug 1315317, bug 1314125 and the backout of bug 1254104 for timeout in devtools/client/responsive.html/test/browser/browser_permission_doorhanger.js: bug 1315317: https://hg.mozilla.org/releases/mozilla-beta/rev/71bc47320df4f7bebe98849a7e625a60f7c584a2 https://hg.mozilla.org/releases/mozilla-beta/rev/55d1d23793f291761d49d5a1b18803f35896aead bug 1314125: https://hg.mozilla.org/releases/mozilla-beta/rev/91496ecb6766e50a43d19304eb1956031406be0a backout of bug 1254104: https://hg.mozilla.org/releases/mozilla-beta/rev/d9c8d4ef148dbfd0ad22b28ec464905f23326dcb Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=1933420&repo=mozilla-beta
Flags: needinfo?(mconley)
Comment 21•7 years ago
|
||
Note that this was a backout of a backout, so essentially, this has re-landed.
Flags: needinfo?(mconley)
You need to log in
before you can comment on or make changes to this bug.
Description
•