Closed Bug 1582396 Opened 2 months ago Closed 2 months ago

The Search bar dropdown doesn’t properly adjust its dimension when it is dragged inside the Overflow Menu

Categories

(Firefox :: Search, defect, P2)

defect
Points:
2

Tracking

()

VERIFIED FIXED
Firefox 71
Iteration:
71.2 - Sept 16 - 29
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox69 --- unaffected
firefox70 --- verified
firefox71 --- verified

People

(Reporter: Anca, Assigned: harry)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(2 files)

Attached video screencast issue.flv

[Affected versions]:

  • Beta 70.0b7
  • Nightly 71.0a1 (2019-09-18)

[Affected platforms]:

  • Windows 10
  • macOS 10.13
  • Ubuntu 18.04

[Steps to reproduce]:

  1. Access the about:preferences#search page
  2. Click the “Add search bar in toolbar” option
  3. Reduce the browser’s width and type something inside the Search bar
  4. Enter the Customize page and drag the Search bar inside the Overflow menu
  5. Open the Overflow Menu and type again something inside the Search bar
  6. Observe the Search bar dropdown

[Expected result]:

  • The Search bar dropdown adjusts its dimension accordingly, to mach the Search bar width.

[Actual result]:

  • The Search bar dropdown doesn’t properly adjust its dimension (all platforms).
  • On Beta the content kept the dimension of the Search bar drowdown before being dragged inside the Overflow Menu (Ubuntu only).

[Regression range]:

[Additional notes]:

  • The issue is partly fixed when entering again the Overflow Menu, the content its properly adjusted, but not the dimension; issue is completely fixed after the browser is restarted - see the attached screencast
  • On Windows/Mac, the content is being properly displayed, only the dimension is not properly adjust.
Has Regression Range: --- → yes
Has STR: --- → yes

Harry, could you please take a look?

Blocks: megabar
Points: --- → 2
Flags: needinfo?(htwyford)
Priority: -- → P2

It looks like this issue stems from how the searchbar sets its autocomplete width in two places. The second approach incorrectly sets min-width too small when the search bar is in the overflow menu. There is a race between the two width-setters: if the searchbar is initialized from the overflow before it is initialized from the toolbar, then the first approach wins and the XUL width attribute takes precedence over the incorrect min-width attribute. After the searchbar is accessed from the toolbar, every subsequent time it is accessed from the overflow menu, the incorrect min-width approach wins.

This bug existed before bug 1561894 landed, but it was hidden because the old search-one-off code padded out the search autocomplete box with dummy items so it took on the correct size.

The min-width setter in autocomplete-popup.js can be totally removed and everything works fine, but I think it makes sense structurally for the searchbar autocomplete class to be able to set its own width, so I've wrapped it in an if check instead to see if it already has a width.

Dao, are there any occasions when autocomplete-popup.js might be loaded when searchbar.js is not? I assume there must be since there are two width-setters. If not, then we could probably just remove the width-setting code from autocomplete-popup.js entirely.

Flags: needinfo?(htwyford) → needinfo?(dao+bmo)

(In reply to Harry Twyford [:harry] from comment #2)

Dao, are there any occasions when autocomplete-popup.js might be loaded when searchbar.js is not? I assume there must be since there are two width-setters. If not, then we could probably just remove the width-setting code from autocomplete-popup.js entirely.

MozSearchAutocompleteRichlistboxPopup from autocomplete-popup.js is only used for this panel: https://searchfox.org/mozilla-central/rev/7531325c8660cfa61bf71725f83501028178cbb9/browser/base/content/browser.xhtml#225

Flags: needinfo?(dao+bmo)
Assignee: nobody → htwyford
Status: NEW → ASSIGNED
Iteration: --- → 71.2 - Sept 16 - 29
Pushed by htwyford@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ed063a8283be
Set width correctly on the searchbar autocomplete in the overflow menu. r=dao
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71

Please nominate this for Beta approval when you get a chance.

Flags: needinfo?(htwyford)

Comment on attachment 9093956 [details]
Bug 1582396 - Set width correctly on the searchbar autocomplete in the overflow menu. r?dao

Beta/Release Uplift Approval Request

  • User impact if declined: The standalone search bar will be too narrow after being moved to the Overflow menu.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: See comment 0
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Worst case is that the standalone search bar continues to be too narrow in the Overflow menu. No core functionality or data is at risk.
  • String changes made/needed:
Flags: needinfo?(htwyford)
Attachment #9093956 - Flags: approval-mozilla-beta?
Flags: qe-verify+

The width of the Search bar is now correctly displayed inside the Overflow menu with Fx 71.0a1 (treeherder build) on Windows 10 x64, macOS 10.13 and Ubuntu 18.04 x64.

Note though, that intermittent I could still reproduced (Ubuntu, Windows) the fact that the content is not adjusting its size accordingly (see screenshot: https://drive.google.com/file/d/1hEKEKnDNz4wQ4f5HgkDHaE8T3eMqy79z/view?usp=sharing).
Also, I see some inconsistencies on Toolbar when playing with the browser’s width. (see screenshot: https://drive.google.com/file/d/1WIdtNDI2SoAi4ruTXVC9uqbak2IlvFcB/view?usp=sharing)

Harry Twyford, any thoughts on this, should these issues be treated separately?

Flags: needinfo?(htwyford)
Blocks: 1584336

Did these issues exist before my fix landed? Or did I introduce them with the attached patch?

In any case, I think we should handle them separately in a follow-up. I've filed it at bug 1584336 and assigned myself.

Flags: needinfo?(htwyford)
Flags: needinfo?(anca.soncutean)
Regressions: 1584336

Also, I see some inconsistencies on Toolbar when playing with the browser’s width. (see screenshot: https://drive.google.com/file/d/1WIdtNDI2SoAi4ruTXVC9uqbak2IlvFcB/view?usp=sharing)

(In reply to Harry Twyford [:harry] from comment #10)

Did these issues exist before my fix landed? Or did I introduce them with the attached patch?

Not sure, but it looks like it, I can reproduce this issue only after the patch landed, using the following steps:

  1. Go to Customize and drag the Search Bar inside the Overflow Menu
  2. Type some characters inside the Search bar
  3. Go to Customize again and drag the Search bar on the Toolbar
  4. Reduce the width of the browser (optional if the browser width is already narrowed) and type something inside the Search bar

The width of the Search bar (on Toolbars) is adjusted after opening the Search bar dropdown second time.

On older builds (before the patch) I’ve tried to reproduce the issue also after I restarted the browser, since the Search bar dropdown was adjusted afterwards (see comment 1, first additional note), but the issue is not reproducible either, using this approach.

Flags: needinfo?(anca.soncutean)

I didn't answered immediately to Harry Twyford's request from comment 10, so I will let a NI for him to be sure it is aware of my response. If anything else that I should test around this issue, let me know. Thank you!

Flags: needinfo?(htwyford)

Hi Anca! I saw your comment. Thank you for the information. A fix for this issue has been pushed in bug 1584336. I will request beta uplift for that patch this week.

Flags: needinfo?(htwyford)

Comment on attachment 9093956 [details]
Bug 1582396 - Set width correctly on the searchbar autocomplete in the overflow menu. r?dao

Fix for UI regression in 70, OK to uplift this with its followup fix in bug 1584336 for beta 12.

Attachment #9093956 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

This issue is no longer reproducible with Fx 71.0a1 (2019-10-07) and Fx 70.0b13 across platforms (Windows 10 x64, macOS 10.13 and Ubuntu 18.04 x64), the Search bar’s width is now correctly adjusted.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.