Closed Bug 1609160 Opened 5 years ago Closed 5 years ago

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)

defect

Tracking

()

VERIFIED FIXED
Firefox 74
Iteration:
74.1 - Jan 6 - Jan 19
Tracking Status
firefox-esr68 --- wontfix
firefox72 --- wontfix
firefox73 + verified
firefox74 --- verified

People

(Reporter: mcoman, Assigned: thecount)

References

(Regression)

Details

(Keywords: regression)

Attachments

(3 files)

Attached image rec of the issue.gif (deleted) —

[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]:

  1. Open the browser with the profile from prerequisites.
  2. Navigate to the "about:preferences#home" page.
  3. Uncheck the "Top Sites" and "Recommended by Pocket" options.
  4. 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]:

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.

Aaron, could you please give us your opinion regarding this?

Flags: needinfo?(abenson)

Yeah this is unexpected as the logo should appear only when the search field is the only visible section.

Flags: needinfo?(abenson)
Assignee: nobody → sdowne

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.

My guess is this is the regression https://bugzilla.mozilla.org/show_bug.cgi?id=1536285

Iteration: --- → 74.1 - Jan 6 - Jan 19
Priority: -- → P1
Pushed by sdowne@getpocket.com:
https://hg.mozilla.org/integration/autoland/rev/5d9ae5b93fca
Search only logo displayed with just highlights. r=gvn
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 74

Please nominate this for Beta approval if you think it's worth taking.

Flags: needinfo?(sdowne)

[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.

Flags: needinfo?(sdowne)

Hi Scott, tracking+ doesn't guarantee uplift. The patch still needs a regular Beta approval request.

Flags: needinfo?(sdowne)

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)

Flags: needinfo?(sdowne)

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:
Attachment #9120826 - Flags: approval-mozilla-beta?
Flags: qe-verify+

(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 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.

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

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
Flags: needinfo?(ryanvm)
Flags: needinfo?(ryanvm) → needinfo?(sdowne)
QA Whiteboard: [qa-triaged]

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.

Status: RESOLVED → VERIFIED

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:
Flags: needinfo?(sdowne)
Attachment #9122163 - Flags: approval-mozilla-beta?

Made a new phabricator pr against an updated beta, should do the trick.

I didn't see how to close my other one.

Comment on attachment 9122163 [details]
Bug 1609160 - Search only logo displayed with just highlights.

Thanks for rebasing. Approved for 73.0b8.

Attachment #9122163 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9120826 - Flags: approval-mozilla-beta+
Regressions: 1610709

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.

Flags: qe-verify+
No longer regressions: 1610709
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: