The "Firefox" name and logo are wrongly displayed above the in-content "Search Bar" even if the "Highlights" section is still enabled
Categories
(Firefox :: New Tab Page, defect, P1)
Tracking
()
People
(Reporter: mcoman, Assigned: thecount)
References
(Regression)
Details
(Keywords: regression)
Attachments
(3 files)
[Affected versions]:
- Firefox Release 72.0.1 - Build ID: 20200107212822
- Firefox Beta 73.0b4 - Build ID: 20200112220634
- Firefox Nightly 74.0a1 - Build ID: 20200114094410
[Affected Platforms]:
- Windows 10 x64
- Mac 10.15.2
- Ubuntu 18.04 x64
[Prerequisites]:
- Have a Firefox profile with the "browser.search.region" pref set to "US" in the "about:config" page.
[Steps to reproduce]:
- Open the browser with the profile from prerequisites.
- Navigate to the "about:preferences#home" page.
- Uncheck the "Top Sites" and "Recommended by Pocket" options.
- Open a new tab and observe the displayed elements.
[Expected result]:
- Only the in-content "Search Bar" and the "Highlights" section are displayed.
[Actual result]:
- The "Firefox" name and logo are also displayed above the in-content "Search Bar".
[Regression Window]:
- Considering the fact that this issue is not reproducible on older Firefox versions, using the Mozregression tool, we have managed to find the following pushlog:
Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=8e8ea33ecb3da138dbd0af56c2e5550902e7b05a&tochange=777492b75f9745bd78dc96155ccd91523b788db0
From the pushlog, it seems that bug 1552289 has caused this behavior.
[Notes]:
- This issue is also reproducible with the "Discovery Steam" experience for the rest of the world.
- This issue is not reproducible on the "control" branch of the experiment.
Reporter | ||
Comment 1•5 years ago
|
||
Aaron, could you please give us your opinion regarding this?
Comment 2•5 years ago
|
||
Yeah this is unexpected as the logo should appear only when the search field is the only visible section.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 3•5 years ago
|
||
I think I see what happened.
Initially, when we did the search only work, highlights wasn't enabled yet in Discovery Stream.
"This is because the search-only view is expecting highlights to be off, but that section doesn't exist in the settings for discovery stream"
So since then, we've integrated highlights, but looks like we forgot to round this part out.
The actual regression bug is probably when we enabled highlights in DS. I'll see if I can find it, but also I'll fix this asap.
Assignee | ||
Comment 4•5 years ago
|
||
My guess is this is the regression https://bugzilla.mozilla.org/show_bug.cgi?id=1536285
Assignee | ||
Comment 5•5 years ago
|
||
Updated•5 years ago
|
Pushed by sdowne@getpocket.com: https://hg.mozilla.org/integration/autoland/rev/5d9ae5b93fca Search only logo displayed with just highlights. r=gvn
Updated•5 years ago
|
Comment 7•5 years ago
|
||
bugherder |
Comment 8•5 years ago
|
||
Please nominate this for Beta approval if you think it's worth taking.
Assignee | ||
Comment 9•5 years ago
|
||
[Tracking Requested - why for this release]: Worth taking because of how small the change is and how low risk it is. It has a noticeable UI improvement for some users.
Comment 10•5 years ago
|
||
Hi Scott, tracking+ doesn't guarantee uplift. The patch still needs a regular Beta approval request.
Assignee | ||
Comment 11•5 years ago
|
||
Ah interesting.
So I knew tracking+ wasn't all that was needed, but I also thought I needed to wait until tracking went from ? to + before taking it to the next step.
So to be clear, it is acceptable for me to create the attachment while still in ? without the + on the approval? (I don't know where I heard that tbh, so it's probably an incorrect assumption on my end)
Assignee | ||
Comment 12•5 years ago
|
||
Comment on attachment 9120826 [details]
Bug 1609160 - Search only logo displayed with just highlights.
Beta/Release Uplift Approval Request
-
User impact if declined: It has a noticeable UI improvement for some users.
-
Is this code covered by automated tests?: Yes
-
Has the fix been verified in Nightly?: No
-
Needs manual test from QE?: Yes
-
If yes, steps to reproduce: [Steps to reproduce]:
Open the browser with the profile from prerequisites.
Navigate to the "about:preferences#home" page.
Uncheck the "Top Sites" and "Recommended by Pocket" options.
Open a new tab and observe the displayed elements.
[Expected result]:
Only the in-content "Search Bar" and the "Highlights" section are displayed.
[Actual result]:
The "Firefox" name and logo are also displayed above the in-content "Search Bar".
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): Worth taking because of how small the change.
- String changes made/needed:
Assignee | ||
Updated•5 years ago
|
Comment 13•5 years ago
|
||
(In reply to Scott [:thecount] Downe from comment #11)
So to be clear, it is acceptable for me to create the attachment while still in ? without the + on the approval? (I don't know where I heard that tbh, so it's probably an incorrect assumption on my end)
Tracking+ basically just means "this is a bug we want to keep on our radar." It's not strictly tied to uplifting in the end since that decision ultimately rests on the relative risks associated with the patch in question. We're certainly more likely to want to uplift patches with tracking status, but it's not a necessary requirement for uplift either.
Comment 14•5 years ago
|
||
Comment on attachment 9120826 [details]
Bug 1609160 - Search only logo displayed with just highlights.
Low-risk fix for a minor visual issue on the New Tab Page. Approved for 73.0b7.
Comment 15•5 years ago
|
||
Conflicts when we tried to uplift this bug:
- conflicts while merging browser/components/newtab/content-src/components/Base/Base.jsx
- conflicts while merging browser/components/newtab/data/content/activity-stream.bundle.js
Updated•5 years ago
|
Updated•5 years ago
|
Reporter | ||
Comment 16•5 years ago
|
||
I have verified that this issue is no longer reproducible with the latest Firefox Nightly (74.0a1 Build ID - 20200119213718) installed, on Windows 10 x64, Ubuntu 18.04 x64 and Mac 10.15.2. Now the "Firefox" name and logo are no longer displayed if the "Highlights" section is enabled.
Assignee | ||
Comment 17•5 years ago
|
||
Assignee | ||
Comment 18•5 years ago
|
||
Comment on attachment 9122163 [details]
Bug 1609160 - Search only logo displayed with just highlights.
Beta/Release Uplift Approval Request
- User impact if declined: It has a noticeable UI improvement for some users.
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: Yes
- If yes, steps to reproduce: Open the browser with the profile from prerequisites.
Navigate to the "about:preferences#home" page.
Uncheck the "Top Sites" and "Recommended by Pocket" options.
Open a new tab and observe the displayed elements.
[Expected result]:
Only the in-content "Search Bar" and the "Highlights" section are displayed.
[Actual result]:
The "Firefox" name and logo are also displayed above the in-content "Search Bar".
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): Worth taking because of how small the change.
- String changes made/needed:
Assignee | ||
Comment 19•5 years ago
|
||
Made a new phabricator pr against an updated beta, should do the trick.
I didn't see how to close my other one.
Comment 20•5 years ago
|
||
Comment on attachment 9122163 [details]
Bug 1609160 - Search only logo displayed with just highlights.
Thanks for rebasing. Approved for 73.0b8.
Updated•5 years ago
|
Comment 21•5 years ago
|
||
bugherder uplift |
Reporter | ||
Comment 22•5 years ago
|
||
I have verified that this issue is no longer reproducible with the latest Firefox Beta (73.0b8 Build ID - 20200122012721) installed, on Windows 10 x64, Ubuntu 18.04 x64 and Mac 10.15.2. Now the "Firefox" name and logo are no longer displayed if the "Highlights" section is enabled.
Reporter | ||
Updated•5 years ago
|
Updated•5 years ago
|
Description
•