1212482 - Large white space in URL Bar because of missing Dropdown

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Mehmet]

Attachment: screenshot.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11) AppleWebKit/601.1.56 (KHTML, like Gecko) Version/9.0 Safari/601.1.56

Steps to reproduce:

Firefox Nightly 44.0a1 (2015-10-07)

Steps to reproduce:

1.) Focus the URL Bar



Actual results:

A large white space.


Expected results:

Not sure what the correct behavior should be.

Why not removing the Dropdown at all?

Comment 1

[Tim Nguyen :ntim]

There is also a large odd white space when the reader button is shown. So we should probably move the url dropmarker to be the left most item and show it when the urlbar is focused.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

The URL dropmarker is already the left-most item. But yeah, showing the dropdown when it is focused would be a simple fix.

Comment 3

[Tim Nguyen :ntim]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #2)
> The URL dropmarker is already the left-most item.
Oh, the problem is only on my profile, I'll investigate which add-on is causing that.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

This fix is marginally better as we will now show the dropmarker when the field is focused, but it will still be a white spot for long URLs when the field is not focused or the navigator-toolbox is not hovered.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/de4fc486564b

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/de4fc486564b

Comment 7

[Dão Gottwald [:dao]]

This appears to have caused a bunch of perf regressions:

Fx-Team-Non-PGO - Tab Animation Test - Ubuntu HW 12.04 x64 (e10s) - 3.56% increase
Fx-Team-Non-PGO - Talos Page Switch - WINNT 6.2 x64 (e10s) - 9.32% increase
Fx-Team-Non-PGO - Tab Animation Test - WINNT 6.2 x64 (e10s) - 11.2% increase
Fx-Team-Non-PGO - Tab Animation Test - WINNT 5.1 (e10s) - 7.54% increase

Comment 8

[Dão Gottwald [:dao]]

backed out: https://hg.mozilla.org/integration/fx-team/rev/2e1cf6a3b3f5

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

At the beginning of each test, TART unfocuses the URL bar, moving focus to the browser.

>  startTest: function(doneCallback, config) {
>    this._onTestComplete = function (results) {
>      Profiler.mark("TART - end", true);
>      doneCallback(results);
>    };
>    this._config = config;
>
>    const Ci = Components.interfaces;
>    var wm = Components.classes["@mozilla.org/appshell/window-mediator;1"].getService(Ci.nsIWindowMediator);
>    this._win = wm.getMostRecentWindow("navigator:browser");
>    this._tartTab = this._win.gBrowser.selectedTab;
>    this._win.gBrowser.selectedBrowser.focus(); // Unfocus the URL bar to avoid caret blink
>
>    Profiler.mark("TART - start", true);
>
>    return this._startTest();
>  }

I believe the regression showing up in comment #7 is due to the fade-out of the dropmarker at the beginning of the test. Avih, should we wait for the dropmarker transition to complete before starting TART or can we set that transition duration to 0 before changing focus?

I don't think it is fair to include the change-of-focus transition as part of TART or Talos Page Switch since this transition only occurs mouse entering/exiting or focus changes.

Comment 10

[Avi Halachmi (:avih)]

> I believe the regression showing up in comment #7 is due to the fade-out of
> the dropmarker at the beginning of the test. Avih, should we wait for the
> dropmarker transition to complete before starting TART or can we set that
> transition duration to 0 before changing focus?

I think the part which you quoted is the unfocus of the "main" TART tab, and it's not part of what's being measured (it's the tab which get focused when all other measured tabs are closed).

For the actual measured animation, there's already 500ms or more idle before each one (depending on sub test).

> I don't think it is fair to include the change-of-focus transition as part
> of TART or Talos Page Switch since this transition only occurs mouse
> entering/exiting or focus changes.

"Fair" is irrelevant for two reasons:

1. We care about one thing - user experience. No test can quantify this into numbers, so it does the next best thing - measure something which is close enough. The numbers are just numbers, and it's up to us humans to interpret how much a change in numbers actually reflects a change in user experience. If you think this specific change is not reflected at all in user experience, just say so.

2. There's no policy to blindly reject patches which affect the results negatively. If the change can be explained and there's a consensus that it's a good change despite the possibly affected numbers (or even proven degradation in user experience), then we're not likely to  object.


However, from what I can tell so far, we have a case where the "base" tab doesn't have the URL bar focused (the patch you quoted), but I'm guessing the newly opened tab does have it focused (which is the default behavior). I think it's actually a very common use case, and if the animation has regressed on this case, then it probably does reflect a real regression in user experience.

So this is probably what you should be focusing on: understand what the case which regressed, understand if it affects actual user experience, and if yes and you still want to keep it, provide some arguments which we can discuss.

Comment 11

[Dão Gottwald [:dao]]

(In reply to Avi Halachmi (:avih) from comment #10)
> However, from what I can tell so far, we have a case where the "base" tab
> doesn't have the URL bar focused (the patch you quoted), but I'm guessing
> the newly opened tab does have it focused (which is the default behavior). I
> think it's actually a very common use case, and if the animation has
> regressed on this case, then it probably does reflect a real regression in
> user experience.

This is my understanding too.

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v2

This patch sets transition-duration to 0s during tabswitch. We could add a different attribute in tabbrowser.xml for browsers that are about to have their focus moved to the location bar (updateCurrentBrowser and _adjustFocusAfterTabSwitch), but this might be enough.

Tryserver run without patch:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf537cc20f10

Tryserver run with patch:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=06a063953f48

Comment 13

[Dão Gottwald [:dao]]

Comment on attachment 8677035 [details] [diff] [review]
Patch v2

>+#urlbar-wrapper[switchingtabs] > #urlbar > .urlbar-textbox-container > .urlbar-history-dropmarker {
>+  transition-duration: 0s;

just use transition: none; here

Comment 15

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/integration/fx-team/rev/0ef32fe804d583ee9a33697f1ce87d53376d8a9d
Bug 1212482 - Don't show a white area at the end of the location bar when not focused. r=dao

Comment 16

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/0ef32fe804d5