Closed Bug 1173866 Opened 5 years ago Closed 5 years ago

DOM fullscreen broken in e10s (DOM fullscreen fails on the other than the first tab)

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Tracking Status
e10s ? ---
firefox41 + verified

People

(Reporter: evilpie, Assigned: xidorn)

References

Details

(Keywords: regression, reproducible)

Attachments

(3 files)

In e10s mode pressing the fullscreen mode button on YouTube just produces a short flash. This is on Linux Mint with the HTML5 player.
Is this a recent regression?
Yes, but it also seems to be specific to my profile, because it works in a clean profile. I however already disabled all my addons in my main profile and it still doesn't work. It's also apparently not specific to YouTube's fullscreen usage, because http://davidwalsh.name/demo/fullscreen.php doesn't work either.
Attached file about:support
I narrowed it down and the range looks really promising!
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d17480aae980&tochange=10d47ad00e7a
Flags: needinfo?(quanxunzhen)
Blocks: 1161802
Summary: youtube fullscreen broken in e10s → fullscreen broken in e10s
I agree it might be a regression of bug 1161802 since that bug makes entering fullscreen asynchronous and adds additional check because of that.

But if you cannot reproduce that with a clean profile, I guess it would be hard to identify where the actual problem is.

Could you provide messages in the Web Console and the Browser Console when you try entering fullscreen? Hopefully they can provide some useful information.
Flags: needinfo?(quanxunzhen) → needinfo?(evilpies)
[Tracking Requested - why for this release]: regression

The problem happens on the other than first tab.

Steps To Reproduce
1. Start Nightly with newly created profile
2. Switch to second tab. If not exist, open new tab
3. Open http://ie.microsoft.com/testdrive/Graphics/VideoFormatSupport/Default.html
        or attached
4. Click fullscreen button

Actual Results:
Fullscreen fails. No error in the Browser Console.

If open the page in the first tab, it works as expected

Pushlog
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d17480aae980&tochange=10d47ad00e7a

Regressed by: Bug 1161802
Attached file DOMFullcreen.html
Summary: fullscreen broken in e10s → DOM fullscreen broken in e10s (DOM fullscreen fails on the other than the first tab)
Flags: needinfo?(quanxunzhen)
Thanks for the steps to reproduce. I can reproduce it now. It is indeed a serious problem.
Flags: needinfo?(evilpies)
Attached patch patchSplinter Review
The problem is because, in bug 1161802 patch 7, we check whether the original target is a browser by checking whether its tag name is "xul:browser". However, for tabs other than the first one, the tag name of browser is just "browser", not "xul:browser". This patch use a different, hopefully more reliable way to distinguish <browser> from other elements.
Assignee: nobody → quanxunzhen
Flags: needinfo?(quanxunzhen)
Attachment #8621409 - Flags: review?(dao)
Attachment #8621409 - Flags: review?(dao) → review+
https://hg.mozilla.org/mozilla-central/rev/d3df23a5ad62

This landed with the wrong bug number.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Oops... sorry about that...
Duplicate of this bug: 1174377
Thanks you very much to fix DOM fullscreen broken.
(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #10)
> However, for tabs other than the first one, the tag name of
> browser is just "browser", not "xul:browser".
Oops, never use tagName, but namespaceURI + localName.
(In reply to Olli Pettay [:smaug] (for generic DOM reviews, you may want to look for other reviewers too ;)) from comment #15)
> (In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #10)
> > However, for tabs other than the first one, the tag name of
> > browser is just "browser", not "xul:browser".
> Oops, never use tagName, but namespaceURI + localName.

Ah... I didn't know that. Thanks.

Then I checked the document of namespaceURI, and saw the example was about checking whether an element is an XUL browser https://developer.mozilla.org/en-US/docs/Web/API/Node/namespaceURI :)

I wonder whether I should back to this way.
Adding a tracking flag for FF41. Setting qe-verify:+ to ensure QE verifies this. This is also noteworthy from a E10s verification point of view.
Flags: qe-verify+
I was able to reproduce this issue on Firefox 41.0a1 (2015-06-11) under Ubuntu 14.04 32-bit.

Verified fixed on Firefox 41.0a1 (2015-06-23) and Firefox 43.0a1 (2015-09-06) using Ubuntu 14.04 32-bit, Mac OS X 10.10.5 and Windows 7 64-bit.
Status: RESOLVED → VERIFIED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.