The default bug view has changed. See this FAQ.

Exiting DOM fullscreen also exits window fullscreen mode

VERIFIED FIXED in Firefox 40

Status

()

Core
DOM
VERIFIED FIXED
3 years ago
a year ago

People

(Reporter: phlsa, Assigned: xidorn)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

Trunk
mozilla40
Points:
5
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify +

Firefox Tracking Flags

(firefox37 wontfix, firefox38 wontfix, firefox39 wontfix, firefox40 verified, firefox-esr31 wontfix, firefox-esr38 wontfix)

Details

(Whiteboard: [qx:spec])

Attachments

(5 attachments, 10 obsolete attachments)

5.74 KB, patch
smaug
: review+
Details | Diff | Splinter Review
7.69 KB, patch
dao
: review+
Details | Diff | Splinter Review
10.76 KB, patch
dao
: review+
Details | Diff | Splinter Review
11.51 KB, patch
dao
: review+
Details | Diff | Splinter Review
2.80 KB, patch
dao
: review+
Details | Diff | Splinter Review
Steps to reproduce:
- Open a page with HTML5 video (e.g. Air Mozilla) in a full screen window
- Toggle full screen mode on the video
- Exit full screen mode of the video (press ESC)

Result:
The entire window exits full screen mode

Expected result:
Just the video should exit full screen mode while the window itself retains full screen.
Comment hidden (obsolete)
Comment hidden (obsolete)
Comment hidden (obsolete)
status-firefox28: --- → affected
status-firefox29: --- → affected
status-firefox30: --- → affected
tracking-firefox28: --- → ?
tracking-firefox29: --- → ?
tracking-firefox30: --- → ?
tracking-firefox28: ? → +
tracking-firefox29: ? → +
tracking-firefox30: ? → +
Comment hidden (obsolete)
To be honest, I don't remember a time when this wasn't an issue.
Ever since Firefox started using the OS X default fullscreen behavior, I've seen this happening.
So I'm not even sure if it is a regression or a less-than-optimal design decision.
If that's the case, and this is Mac-specific (small percentage of overall users) it's not a release blocker but if there's a low risk fix made available, please go ahead with uplift nominations and we can take it up to channels depending on timing & risk.
tracking-firefox28: ? → -
tracking-firefox29: ? → -
tracking-firefox30: ? → -
Flags: firefox-backlog?

Updated

3 years ago
Flags: firefox-backlog? → firefox-backlog+

Updated

3 years ago
Whiteboard: p=5

Updated

3 years ago
Duplicate of this bug: 940841

Updated

3 years ago
Duplicate of this bug: 997724

Updated

3 years ago
Duplicate of this bug: 745520

Comment 10

3 years ago
Not marking bug 807395 as a duplicate of this one, since I'm not sure if this bug is platform specific or not.
See Also: → bug 807395

Updated

3 years ago
Duplicate of this bug: 1075385

Updated

2 years ago
Points: --- → 5
Whiteboard: p=5
Whiteboard: [qx]
Depends on: 1121280
Blocks: 1121280
No longer depends on: 1121280
(Assignee)

Comment 12

2 years ago
This is not a Mac-only problem. It happens on Windows as well. Given the current code for DOM fullscreen highly relies on the fullscreen mode code, I reasonably assume it is a problem affects all platforms.

To solve this, we have to make those two parts work (mostly) independently.
OS: Mac OS X → All
Hardware: x86 → All
Summary: [Mac] Exiting full screen video playback (HTML5) also exits window full screen mode → Exiting full screen video playback (HTML5) also exits window full screen mode
Version: 28 Branch → Trunk
(Assignee)

Updated

2 years ago
Depends on: 1123170

Updated

2 years ago
status-firefox35: --- → ?

Comment 13

2 years ago
[Tracking Requested - why for this release]:
status-firefox38: --- → ?
tracking-firefox36: --- → ?
Comment hidden (me-too)
Comment hidden (me-too)

Updated

2 years ago
Flags: qe-verify+

Updated

2 years ago
status-firefox39: --- → ?

Comment 16

2 years ago
This applies not only to video, but also on HTML5 Canvas. True all platforms? My friends did not see the problem.
(Assignee)

Comment 17

2 years ago
(In reply to RblSb@nm.ru from comment #16)
> This applies not only to video, but also on HTML5 Canvas. True all
> platforms? My friends did not see the problem.

All platforms. He is probably using flash to play video, which is not affected by this bug. Flash has its own fullscreen mechanism.
Keywords: regressionwindow-wanted
Summary: Exiting full screen video playback (HTML5) also exits window full screen mode → Exiting DOM fullscreen also exits window fullscreen mode

Updated

2 years ago
status-firefox40: --- → ?
(Assignee)

Updated

2 years ago
Assignee: nobody → quanxunzhen
(Assignee)

Updated

2 years ago
status-firefox28: affected → ---
status-firefox29: affected → ---
status-firefox30: affected → ---
status-firefox35: affected → ---
status-firefox36: affected → ---
status-firefox39: ? → affected
status-firefox40: ? → affected
status-firefox-esr31: --- → affected
status-firefox-esr38: --- → affected
tracking-firefox36: - → ---
(Assignee)

Updated

2 years ago
tracking-firefox28: - → ---
tracking-firefox29: - → ---
tracking-firefox30: - → ---
(Assignee)

Updated

2 years ago
See Also: bug 807395
(Assignee)

Updated

2 years ago
Duplicate of this bug: 807395
(Assignee)

Comment 19

2 years ago
Created attachment 8589954 [details] [diff] [review]
patch 1 - simplify code
Attachment #8589954 - Flags: review?(dao)
(Assignee)

Comment 20

2 years ago
Created attachment 8589956 [details] [diff] [review]
patch 2 - fix this bug
Attachment #8589956 - Flags: review?(dao)
Attachment #8589956 - Flags: review?(bugs)
Attachment #8589956 - Flags: feedback?(cpearce)
(Assignee)

Comment 21

2 years ago
Created attachment 8589957 [details] [diff] [review]
patch 3 - test
Attachment #8589957 - Flags: review?(bugs)
Comment on attachment 8589956 [details] [diff] [review]
patch 2 - fix this bug

>+    nsRefPtr<AsyncEventDispatcher> asyncDispatcher = new AsyncEventDispatcher(
>+        this, NS_LITERAL_STRING("MozExitedDomFullscreen"), true, true);
Nit, 2 space indentation, please
Attachment #8589956 - Flags: review?(bugs) → review+
Comment on attachment 8589957 [details] [diff] [review]
patch 3 - test

So this tests the events. 
Don't we want also actual browser chrome test too
(which dao should probably review).
Attachment #8589957 - Flags: review?(bugs) → review+
(Assignee)

Comment 24

2 years ago
Created attachment 8590553 [details] [diff] [review]
patch 4 - browser mochitest

I suspect this test would probably fail on Mac...
Attachment #8590553 - Flags: review?(dao)
(Assignee)

Comment 25

2 years ago
Comment on attachment 8590553 [details] [diff] [review]
patch 4 - browser mochitest

need to find out why this test cause failures first...

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf2c153849cb
Attachment #8590553 - Flags: review?(dao)
(Assignee)

Updated

2 years ago
Attachment #8589954 - Flags: review?(dao)
(Assignee)

Updated

2 years ago
Attachment #8589956 - Flags: review?(dao)
(Assignee)

Comment 26

2 years ago
Comment on attachment 8589954 [details] [diff] [review]
patch 1 - simplify code

This doesn't seem to cause the problems, hence request for review again.
Attachment #8589954 - Flags: review?(dao)
(Assignee)

Comment 27

2 years ago
Created attachment 8591477 [details] [diff] [review]
patch 2 - fix this bug, r=smaug
Attachment #8589956 - Attachment is obsolete: true
Attachment #8589956 - Flags: feedback?(cpearce)
Attachment #8591477 - Flags: review?(dao)
Attachment #8591477 - Flags: feedback?(cpearce)
(Assignee)

Comment 28

2 years ago
Created attachment 8591484 [details] [diff] [review]
patch 4 - browser mochitest

The only left problems are that this test fails on Linux without e10s, and Windows & Mac with e10s: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9b6822a36c7c

I really have no idea what I can do to fix those failures. Any suggestions?
Flags: needinfo?(cpearce)
(Assignee)

Updated

2 years ago
Depends on: 1154479
(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #28)
> Created attachment 8591484 [details] [diff] [review]
> patch 4 - browser mochitest
> 
> The only left problems are that this test fails on Linux without e10s, and
> Windows & Mac with e10s:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=9b6822a36c7c
> 
> I really have no idea what I can do to fix those failures. Any suggestions?

Dunno. You must have a race somewhere.
Flags: needinfo?(cpearce)
(Assignee)

Comment 30

2 years ago
Finally! \o/ https://treeherder.mozilla.org/#/jobs?repo=try&revision=8966ed960679

Will submit the revised patch tomorrow.
(Assignee)

Comment 31

2 years ago
Created attachment 8593637 [details] [diff] [review]
patch 4 - browser mochitest
Attachment #8590553 - Attachment is obsolete: true
Attachment #8591484 - Attachment is obsolete: true
Attachment #8593637 - Flags: review?(dao)

Comment 32

2 years ago
(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #31)
> Created attachment 8593637 [details] [diff] [review]
> patch 4 - browser mochitest

Man, I love youuuuu. Looked today the latest nightly build, where the problem is still there. When you will see a fix? Really want :3
(Assignee)

Comment 33

2 years ago
(In reply to RblSb@nm.ru from comment #32)
> (In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #31)
> > Created attachment 8593637 [details] [diff] [review]
> > patch 4 - browser mochitest
> 
> Man, I love youuuuu. Looked today the latest nightly build, where the
> problem is still there. When you will see a fix? Really want :3

When I get my patches granted from :dao.
I'll try to get to this tomorrow.
Comment on attachment 8589954 [details] [diff] [review]
patch 1 - simplify code

>--- a/browser/base/content/browser-fullScreen.js
>+++ b/browser/base/content/browser-fullScreen.js
>@@ -68,25 +68,21 @@ var FullScreen = {
> 
>     if (enterFS) {
>       document.addEventListener("keypress", this._keyToggleCallback, false);
>       document.addEventListener("popupshown", this._setPopupOpen, false);
>       document.addEventListener("popuphidden", this._setPopupOpen, false);
>       // We don't animate the toolbar collapse if in DOM full-screen mode,
>       // as the size of the content area would still be changing after the
>       // mozfullscreenchange event fired, which could confuse content script.
>-      this._shouldAnimate = !document.mozFullScreen;
>-      this.mouseoverToggle(false);
>+      this._shouldAnimate = true;
>+      this.hideNavToolbox(document.mozFullScreen);

The above comment was referring to this._shouldAnimate = !document.mozFullScreen. Now it doesn't seem to make sense anymore.

>+  hideNavToolbox: function(forceHide)
>+  {

While you're at it, please put these on one line:

hideNavToolbox: function(forceHide) {

>+#main-window[inDOMFullscreen] #navigator-toolbox {
>+  display: none;
>+}

I'm not sure what this is about. You're still calling hideNavToolbox for DOM fullscreen mode anyway, aren't you?

>+#main-window[inFullscreen] #navigator-toolbox[shouldAnimate] {
>+  transition: 1.5s margin-top ease-out;
>+}

* the descendent selector isn't needed here, as the inFullscreen is also present on #navigator-toolbox

* please rename shouldAnimate to something more fullscreen-specific

* will this disable the margin-left and margin-right transitions when entering customize mode from fullscreen? http://hg.mozilla.org/mozilla-central/annotate/946ac85af8f4/browser/base/content/browser.css#l1087
Attachment #8589954 - Flags: review?(dao)
(Assignee)

Comment 36

2 years ago
(In reply to Dão Gottwald [:dao] from comment #35)
> Comment on attachment 8589954 [details] [diff] [review]
> patch 1 - simplify code
> >+#main-window[inDOMFullscreen] #navigator-toolbox {
> >+  display: none;
> >+}
> 
> I'm not sure what this is about. You're still calling hideNavToolbox for DOM
> fullscreen mode anyway, aren't you?

Yes, but I want to ensure that the toolbox won't appear accidently.

> >+#main-window[inFullscreen] #navigator-toolbox[shouldAnimate] {
> >+  transition: 1.5s margin-top ease-out;
> >+}
> 
> * please rename shouldAnimate to something more fullscreen-specific

What about adding a prefix, say fullscreenShouldAnimate?

> * will this disable the margin-left and margin-right transitions when
> entering customize mode from fullscreen?
> http://hg.mozilla.org/mozilla-central/annotate/946ac85af8f4/browser/base/
> content/browser.css#l1087

Probably yes in some case. Normally it won't. I guess I can just add an event listener to remove that attribute once the transition finishes.
(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #36)
> (In reply to Dão Gottwald [:dao] from comment #35)
> > Comment on attachment 8589954 [details] [diff] [review]
> > patch 1 - simplify code
> > >+#main-window[inDOMFullscreen] #navigator-toolbox {
> > >+  display: none;
> > >+}
> > 
> > I'm not sure what this is about. You're still calling hideNavToolbox for DOM
> > fullscreen mode anyway, aren't you?
> 
> Yes, but I want to ensure that the toolbox won't appear accidently.

That would mean the code is broken, wouldn't it? We should fix that, if needed, rather than having two pieces of code doing the same thing...

> > >+#main-window[inFullscreen] #navigator-toolbox[shouldAnimate] {
> > >+  transition: 1.5s margin-top ease-out;
> > >+}
> > 
> > * please rename shouldAnimate to something more fullscreen-specific
> 
> What about adding a prefix, say fullscreenShouldAnimate?

ok.
(Assignee)

Comment 38

2 years ago
(In reply to Dão Gottwald [:dao] from comment #37)
> (In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #36)
> > (In reply to Dão Gottwald [:dao] from comment #35)
> > > Comment on attachment 8589954 [details] [diff] [review]
> > > patch 1 - simplify code
> > > >+#main-window[inDOMFullscreen] #navigator-toolbox {
> > > >+  display: none;
> > > >+}
> > > 
> > > I'm not sure what this is about. You're still calling hideNavToolbox for DOM
> > > fullscreen mode anyway, aren't you?
> > 
> > Yes, but I want to ensure that the toolbox won't appear accidently.
> 
> That would mean the code is broken, wouldn't it? We should fix that, if
> needed, rather than having two pieces of code doing the same thing...

Well, actually just a double assurance. I think it should be fine to just remove the CSS code here without breaking anything.
(Assignee)

Comment 39

2 years ago
(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #36)
> (In reply to Dão Gottwald [:dao] from comment #35)
> > Comment on attachment 8589954 [details] [diff] [review]
> > patch 1 - simplify code
> > * will this disable the margin-left and margin-right transitions when
> > entering customize mode from fullscreen?
> > http://hg.mozilla.org/mozilla-central/annotate/946ac85af8f4/browser/base/
> > content/browser.css#l1087
> 
> Probably yes in some case. Normally it won't. I guess I can just add an
> event listener to remove that attribute once the transition finishes.

I guess it is not necessary. Is it possible to enter customize mode from fullscreen when the toolbox is hidden? Only that case could cause the transition be disabled. When the toolbox is shown, that attribute is never set.
(Assignee)

Comment 40

2 years ago
Created attachment 8596252 [details] [diff] [review]
patch 1 - simplify code
Attachment #8589954 - Attachment is obsolete: true
Attachment #8596252 - Flags: review?(dao)
Comment on attachment 8596252 [details] [diff] [review]
patch 1 - simplify code

>+#main-window[inFullscreen] #navigator-toolbox[fullscreenShouldAnimate] {
>+  transition: 1.5s margin-top ease-out;
>+}

As mentioned in comment 35, please get rid of the descendent selector.

This patch appears to make the "fullscreen toggler" (the thin black bar for unhiding toolbars) visible while the toolbox animates...
Attachment #8596252 - Flags: review?(dao) → review-
(Assignee)

Comment 42

2 years ago
Is there any concern if I remove the fullscreen toggler and use MousePosTracker instead?
Flags: needinfo?(dao)
(Assignee)

Comment 43

2 years ago
Created attachment 8596976 [details] [diff] [review]
patch 1 - simplify code
Attachment #8596252 - Attachment is obsolete: true
Flags: needinfo?(dao)
Attachment #8596976 - Flags: review?(dao)
Comment on attachment 8596976 [details] [diff] [review]
patch 1 - simplify code

>+    if (this._shouldAnimate && !forceHide) {
>+      gNavToolbox.setAttribute("fullscreenShouldAnimate", true);
>+      this._shouldAnimate = false;
>+      // Hide the fullscreen toggler until the transition ends.
>+      gNavToolbox.addEventListener("transitionend", function listener() {
>+        gNavToolbox.removeEventListener("transitionend", listener, true);
>+        if (self._isChromeCollapsed)
>+          self._fullScrToggler.hidden = false;
>+      }, true);
>+      self._fullScrToggler.hidden = true;
>+    }

Please use an arrow function:

let listener = event => {
  [...]
};
gNavToolbox.addEventListener("transitionend", listener, true);

... and use 'this' instead of 'self'.
Attachment #8596976 - Flags: review?(dao)
(Assignee)

Comment 45

2 years ago
(In reply to Dão Gottwald [:dao] from comment #44)
> Comment on attachment 8596976 [details] [diff] [review]
> patch 1 - simplify code
> 
> >+    if (this._shouldAnimate && !forceHide) {
> >+      gNavToolbox.setAttribute("fullscreenShouldAnimate", true);
> >+      this._shouldAnimate = false;
> >+      // Hide the fullscreen toggler until the transition ends.
> >+      gNavToolbox.addEventListener("transitionend", function listener() {
> >+        gNavToolbox.removeEventListener("transitionend", listener, true);
> >+        if (self._isChromeCollapsed)
> >+          self._fullScrToggler.hidden = false;
> >+      }, true);
> >+      self._fullScrToggler.hidden = true;
> >+    }
> 
> Please use an arrow function:
> 
> let listener = event => {
>   [...]
> };
> gNavToolbox.addEventListener("transitionend", listener, true);
> 
> ... and use 'this' instead of 'self'.

I don't think 'this' can be used here. If you don't like 'self', then probably 'FullScreen' should be used instead.
(Assignee)

Comment 46

2 years ago
Created attachment 8597051 [details] [diff] [review]
patch 1 - simplify code
Attachment #8596976 - Attachment is obsolete: true
Attachment #8597051 - Flags: review?(dao)
(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #45)
> (In reply to Dão Gottwald [:dao] from comment #44)
> > Please use an arrow function:
> > 
> > let listener = event => {
> >   [...]
> > };
> > gNavToolbox.addEventListener("transitionend", listener, true);
> > 
> > ... and use 'this' instead of 'self'.
> 
> I don't think 'this' can be used here.

It can.
Comment on attachment 8597051 [details] [diff] [review]
patch 1 - simplify code

This patch also seems to contain a bunch of debug logging that we don't want to land.
Attachment #8597051 - Flags: review?(dao) → review-
(Assignee)

Comment 49

2 years ago
(In reply to Dão Gottwald [:dao] from comment #48)
> Comment on attachment 8597051 [details] [diff] [review]
> patch 1 - simplify code
> 
> This patch also seems to contain a bunch of debug logging that we don't want
> to land.

Sorry about forgetting to remove the debug loggings. But those loggings are easy to remove. Please do not block on things which can be fixed simply. Could you tell me if there is any other problem? If no, could you continue to review the remining patches?
Comment on attachment 8591477 [details] [diff] [review]
patch 2 - fix this bug, r=smaug

This doesn't properly apply on top of the current patch 1. It will probably need an update once patch 1 is done. That's why I was waiting with the review of this patch.
Attachment #8591477 - Flags: review?(dao)
(Assignee)

Comment 51

2 years ago
Created attachment 8597114 [details] [diff] [review]
patch 1 - simplify code
Attachment #8597051 - Attachment is obsolete: true
Attachment #8597114 - Flags: review?(dao)
(Assignee)

Comment 52

2 years ago
Created attachment 8597119 [details] [diff] [review]
patch 2 - fix this bug, r=smaug

My fault. I forgot I've updated this patch locally before for a change from the trunk.
Attachment #8591477 - Attachment is obsolete: true
Attachment #8591477 - Flags: feedback?(cpearce)
Attachment #8597119 - Flags: review?(dao)
Comment on attachment 8597114 [details] [diff] [review]
patch 1 - simplify code

>+      let listener = () => {
>+        gNavToolbox.removeEventListener("transitionend", listener, true);
>+        if (FullScreen._isChromeCollapsed)
>+          FullScreen._fullScrToggler.hidden = false;
>+      };

'this' works perfectly fine here. That's actually the only reason why I asked you to use an arrow function. See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions#Lexical_this
Attachment #8597114 - Flags: review?(dao)
(Assignee)

Comment 54

2 years ago
Created attachment 8597128 [details] [diff] [review]
patch 1 - simplify code

Oh, ok, thanks. Interesting. I didn't know that.
Attachment #8597114 - Attachment is obsolete: true
Attachment #8597128 - Flags: review?(dao)
Comment on attachment 8597128 [details] [diff] [review]
patch 1 - simplify code

>+  showNavToolbox: function(trackMouse) {

How about making trackMouse the default so that callers can opt out only when they really want that:

showNavToolbox: function(trackMouse = true) {

> function focusAndSelectUrlBar() {
>   if (gURLBar) {
>     if (window.fullScreen)
>-      FullScreen.mouseoverToggle(true);
>+      FullScreen.showNavToolbox();
> 
>     gURLBar.select();
>     if (document.activeElement == gURLBar.inputField)
>       return true;
>   }
>   return false;
> }

Not tracking the mouse here seems like a bad behavior change.

STR:
* Go fullscreen, wait for toolbars to hide
* Press Accel+L
* Unfocus the location bar by clicking in the content area
* Move the mouse to the top of the screen and back over the content area
Attachment #8597128 - Flags: review?(dao) → review-
(Assignee)

Comment 56

2 years ago
Created attachment 8597210 [details] [diff] [review]
patch 1 - simplify code

Hmmm... JavaScript has added a lot of interesting things.
Attachment #8597128 - Attachment is obsolete: true
Attachment #8597210 - Flags: review?(dao)

Updated

2 years ago
Attachment #8597210 - Flags: review?(dao) → review+
Comment on attachment 8593637 [details] [diff] [review]
patch 4 - browser mochitest

>+  window.focus();
>+  browser.focus();

I don't think these are needed, the window and the browser being focused is the expected default behavior here.
Attachment #8593637 - Flags: review?(dao) → review+

Updated

2 years ago
Attachment #8597119 - Flags: review?(dao) → review+
(Assignee)

Comment 58

2 years ago
(In reply to Dão Gottwald [:dao] from comment #57)
> Comment on attachment 8593637 [details] [diff] [review]
> patch 4 - browser mochitest
> 
> >+  window.focus();
> >+  browser.focus();
> 
> I don't think these are needed, the window and the browser being focused is
> the expected default behavior here.

IIRC, without these calls, the test may intermittent timeout in some platforms. But I'll push a try without them to see whether it works fine.
(Assignee)

Comment 59

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5972bbe25650
(Assignee)

Comment 60

2 years ago
Hmmmm.... A perm-fail in another bc test with Linux debug builds on e10s...
(Assignee)

Comment 61

2 years ago
I cannot reproduce that failure in my local linux debug build, hence I have no clear idea how to fix that failure. However, I do see some weird behavior when it runs after the test before it, which is browser_885052_customize_mode_observers_disabed.js. 

The test for 885052 is related to fullscreen. When those tests are run sequentially, the windows created in test 885530 (the failing test above) may be maximized, which are in normal mode when it is run individually. This behavior can be seen without this patchset, hence not actually a regression from my patches.

But I suspect that the weird behavior may relate to the failure, and it seems disabling test 885052 on linux e10s does make things work: https://treeherder.mozilla.org/#/jobs?repo=try&revision=48cbc6d990a1

So could I disable that, and leave it to be investigated later?
Flags: needinfo?(dao)
(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #61)
> I cannot reproduce that failure in my local linux debug build, hence I have
> no clear idea how to fix that failure. However, I do see some weird behavior
> when it runs after the test before it, which is
> browser_885052_customize_mode_observers_disabed.js. 
> 
> The test for 885052 is related to fullscreen. When those tests are run
> sequentially, the windows created in test 885530 (the failing test above)
> may be maximized, which are in normal mode when it is run individually. This
> behavior can be seen without this patchset, hence not actually a regression
> from my patches.
> 
> But I suspect that the weird behavior may relate to the failure, and it
> seems disabling test 885052 on linux e10s does make things work:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=48cbc6d990a1
> 
> So could I disable that, and leave it to be investigated later?

Deferring to Jared who wrote that test.
Flags: needinfo?(dao) → needinfo?(jaws)
(Assignee)

Comment 63

2 years ago
Created attachment 8601410 [details] [diff] [review]
patch 0 - modify test of bug 885052

This patch fix the perm-fail on e10s bc1: https://treeherder.mozilla.org/#/jobs?repo=try&revision=37be1a734144
Flags: needinfo?(jaws)
Attachment #8601410 - Flags: review?(dao)
(Assignee)

Comment 64

2 years ago
I was sending the request to the original reviewer of that test, :Unfocused. However it seems he's currently not reviewing patches (and blocks review requests...) So, dao, could you review that?

Updated

2 years ago
Attachment #8601410 - Flags: review?(dao) → review+
(Assignee)

Comment 65

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c08fd6349e11

Comment 66

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/373a497c69ca
https://hg.mozilla.org/integration/mozilla-inbound/rev/f1cd052c7fb7
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ac24fe93a88
https://hg.mozilla.org/integration/mozilla-inbound/rev/e40fca3b4a54
https://hg.mozilla.org/integration/mozilla-inbound/rev/f5c29147aa66
https://hg.mozilla.org/mozilla-central/rev/373a497c69ca
https://hg.mozilla.org/mozilla-central/rev/f1cd052c7fb7
https://hg.mozilla.org/mozilla-central/rev/4ac24fe93a88
https://hg.mozilla.org/mozilla-central/rev/e40fca3b4a54
https://hg.mozilla.org/mozilla-central/rev/f5c29147aa66
Status: NEW → RESOLVED
Last Resolved: 3 years ago2 years ago
status-firefox40: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40

Updated

2 years ago
Iteration: --- → 40.3 - 11 May
(Assignee)

Comment 68

2 years ago
I guess I probably should have landed it in fx-team instead of mozilla-inbound. I'm not sure what's the difference between those two repos. It doesn't seem to be a big issue, though.

Comment 69

2 years ago
Noticed this landed while working on something else. I suspect this may have fixed some other issues with this transition. Setting needinfo so I can look later.
Flags: needinfo?(gijskruitbosch+bugs)

Updated

2 years ago
Flags: needinfo?(gijskruitbosch+bugs)

Comment 70

2 years ago
After exiting fullscreen site set to full screen and hides all the panels Firefox, but you can show the panel by pointing the mouse up. Interesting bug :) OSX 10.9, last FF build
(Assignee)

Updated

2 years ago
Depends on: 1162752

Comment 71

2 years ago
Can not you fix it at least FF v.39? In this case, we have to wait another month v.40 and v.41 some even (for example I D:). The FF v.38 on YouTube completely removed flash player. Use html5 intolerable in any version of the browser -_- I drink vodka and crying
(Assignee)

Comment 72

2 years ago
(In reply to RblSb@nm.ru from comment #71)
> Can not you fix it at least FF v.39? In this case, we have to wait another
> month v.40 and v.41 some even (for example I D:). The FF v.38 on YouTube
> completely removed flash player. Use html5 intolerable in any version of the
> browser -_- I drink vodka and crying

This patchset is non-trivial, and also caused regression (bug 1162752). It is probably too risky to get the approval for uplifting them to Beta. Sorry.
(Assignee)

Updated

2 years ago
Blocks: 1105939
Whiteboard: [qx] → [qx:spec]

Updated

2 years ago
Depends on: 1176519

Updated

2 years ago
status-firefox-esr31: affected → wontfix
status-firefox-esr38: affected → wontfix

Updated

2 years ago
status-firefox37: affected → wontfix
status-firefox38: affected → wontfix
status-firefox39: affected → wontfix
(Assignee)

Updated

2 years ago
Depends on: 1179123
Duplicate of this bug: 1182845
Blocks: 545812
Duplicate of this bug: 787696
Duplicate of this bug: 864378
Duplicate of this bug: 895759
I was able to reproduce this issue on Firefox 40.0a1 (2015-05-01) under Windows 7 64-bit.

Verified fixed on Firefox 40 Beta 4 (20150713153304) under Windows 7 64-bit, Ubuntu 14.04 32-bit and Mac OS X 10.9.5.
Status: RESOLVED → VERIFIED
status-firefox40: fixed → verified

Updated

a year ago
Depends on: 1226297
You need to log in before you can comment on or make changes to this bug.