Closed
Bug 1133583
Opened 10 years ago
Closed 10 years ago
EME error notifications should pass the window to which they apply
Categories
(Core :: Audio/Video, defect)
Core
Audio/Video
Tracking
()
People
(Reporter: Gijs, Assigned: Gijs)
References
Details
Attachments
(2 files)
(deleted),
patch
|
cpearce
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
lmandel
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
Obviously I should have just noticed this in the review for bug 1111160. Sorry.
On the upside, I have a patch!
Assignee | ||
Comment 1•10 years ago
|
||
This seems to work in my local testing. Does this look right?
Attachment #8565149 -
Flags: review?(cpearce)
Comment 2•10 years ago
|
||
Comment on attachment 8565149 [details] [diff] [review]
pass window in EME notifications instead of null subject,
Review of attachment 8565149 [details] [diff] [review]:
-----------------------------------------------------------------
I trust that passing a window reference across process boundaries will somehow magically work with e10s enabled?
Attachment #8565149 -
Flags: review?(cpearce) → review+
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to Chris Pearce (:cpearce) from comment #2)
> Comment on attachment 8565149 [details] [diff] [review]
> pass window in EME notifications instead of null subject,
>
> Review of attachment 8565149 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> I trust that passing a window reference across process boundaries will
> somehow magically work with e10s enabled?
No, the observer service doesn't do anything cross-window boundaries, AIUI (ie the notification will only fire in the same process as where notifyObservers is called, being the content process in our case). The browser code takes care of lobbing the required info across to the parent process based on the message manager for the relevant window - it already does this. :-)
Assignee | ||
Comment 4•10 years ago
|
||
Comment 5•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox38:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Updated•10 years ago
|
Flags: qe-verify?
Flags: firefox-backlog+
Assignee | ||
Updated•10 years ago
|
Flags: qe-verify? → qe-verify-
Updated•10 years ago
|
Blocks: eme-platform-uplift
Comment 6•10 years ago
|
||
status-firefox37:
--- → fixed
Comment 7•10 years ago
|
||
Patch for beta branch as part of EME platform uplift.
Comment 8•10 years ago
|
||
Comment on attachment 8572376 [details] [diff] [review]
Beta patch
Requesting retroactive approval for Beta landing as part of EME platform uplift.
Attachment #8572376 -
Flags: approval-mozilla-beta?
Comment 9•10 years ago
|
||
Comment on attachment 8572376 [details] [diff] [review]
Beta patch
Approved for Beta as part of EME platform uplift.
Attachment #8572376 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in
before you can comment on or make changes to this bug.
Description
•