The new bar while stay in place in full screen if you press CTRL + T

RESOLVED FIXED in Firefox 64

Status

()

defect
P1
normal
RESOLVED FIXED
Last year
6 months ago

People

(Reporter: michaelziliang, Assigned: xidorn)

Tracking

62 Branch
Firefox 64
Points:
---

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0
Build ID: 20180719140244

Steps to reproduce:

1. Enter full screen
2. Press CTRL + T to open a new tab


Actual results:

The new bar while stay in place in full screen even after restarting Firefox


Expected results:

The new tab bar should have disapeared after a few seconds/moving your mouse of the new tab bar.
this relates to fullscreen, ni :xidorn
Flags: needinfo?(xidorn+moz)
So this is because when new tab is created, the focus is on the url bar, in which case we would not collapse the toolbar.

But we should still collapse it as far as the input loses the focus. I think I have some idea about how to fix it.
Assignee: nobody → xidorn+moz
Status: UNCONFIRMED → NEW
Component: Untriaged → General
Ever confirmed: true
Flags: needinfo?(xidorn+moz)
[triage] :xidorn, to clarify this is f11 / browser full screen, and the "new bar" is the main tab bar? 
Assigning p1 as this has a patch up for review.
Priority: -- → P1
I think the "new bar" means the whole toolbox. I'm not completely sure but that feels like the only possible thing given the description.
Comment on attachment 8994077 [details]
Bug 1477525 part 2 - Try hiding toolbox again when input in chrome loses the focus.

https://reviewboard.mozilla.org/r/258662/#review269218

::: browser/base/content/browser-fullScreenAndPointerLock.js:606
(Diff revision 1)
> -        document.commandDispatcher.focusedElement.localName == "input") {
> +        focused.localName == "input") {
> +      // But try collapse the chrome again when it loses the focus.
> +      focused.addEventListener("blur", () => {
> +        // If we don't set any timeout here, the content may move before
> +        // user's click takes effect.
> +        setTimeout(() => this.hideNavToolbox(aAnimate), 300);

This is rather fragile as it assumes that a click will be done after 300ms. Technically I don't think there's any limit for how long a click can take. Is there a more robust way to detect this?
Attachment #8994077 - Flags: review?(dao+bmo)
Comment on attachment 8994076 [details]
Bug 1477525 part 1 - Merge the logic of _safeToCollapse into hideNavToolbox.

https://reviewboard.mozilla.org/r/258664/#review269220
Attachment #8994076 - Flags: review?(dao+bmo) → review+
Comment on attachment 8994077 [details]
Bug 1477525 part 2 - Try hiding toolbox again when input in chrome loses the focus.

https://reviewboard.mozilla.org/r/258662/#review269218

> This is rather fragile as it assumes that a click will be done after 300ms. Technically I don't think there's any limit for how long a click can take. Is there a more robust way to detect this?

I admit that it is rather fragile, but I don't think it's a problem in general.

The consideration is that the UI shouldn't change in a way that user cannot realize what's happening. However, if the user deliberately presses the button for long, it is probably not a big problem if the UI has changed when the button's up.

There are several alternatives I guess:
1. always animate the hiding in this case, so that the UI wouldn't change dramatically in a short time
2. track whether a click is on the way as some global state
3. listen on specific events (e.g. keypress, click) for this check rather than using blur

(2) feels fragile as well, and (3) is harder to be reliable. (1) may work although we may still want to wait for a random time before start animating.

Any other suggestion?
Flags: needinfo?(dao+bmo)
Attachment #8994077 - Attachment is obsolete: true
Attachment #8994076 - Attachment is obsolete: true
MozReview-Commit-ID: LnwVXN4oS2Z

Depends on D6750
Flags: needinfo?(dao+bmo)
I'm losing my development environment to test this patches in several days, so it would be really appreciated if you can review the patches soonish, otherwise I may not be able to fix and land it if there is any issue.
Flags: needinfo?(dao+bmo)
It doesn't matter now. I've wiped the machine.
Flags: needinfo?(dao+bmo)
Comment on attachment 9011671 [details]
Bug 1477525 part 1 - Merge the logic of _safeToCollapse into hideNavToolbox. r=dao

Dão Gottwald [::dao] has approved the revision.
Attachment #9011671 - Flags: review+
Comment on attachment 9011672 [details]
Bug 1477525 part 2 - Try hiding toolbox again when input in chrome loses the focus. r=dao

Dão Gottwald [::dao] has approved the revision.
Attachment #9011672 - Flags: review+
Thanks.
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1ecda91f00bf
part 1 - Merge the logic of _safeToCollapse into hideNavToolbox. r=dao
https://hg.mozilla.org/integration/autoland/rev/cdedc6a6b40a
part 2 - Try hiding toolbox again when input in chrome loses the focus. r=dao
https://hg.mozilla.org/mozilla-central/rev/1ecda91f00bf
https://hg.mozilla.org/mozilla-central/rev/cdedc6a6b40a
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
I have reproduced this bug with Nightly 63.0a1 (2018-07-21) on Windows 7, 64 Bit!
This bug's fix is verified with latest Beta!

Build ID 	20181122182000
User Agent 	Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:64.0) Gecko/20100101 Firefox/64.0
QA Whiteboard: [bugday-20181121]
Duplicate of this bug: 1195927
You need to log in before you can comment on or make changes to this bug.