Closed Bug 1584281 Opened 5 years ago Closed 5 years ago

Update megabar box-shadow and add the same box-shadow to the legacy (non-megabar) results panel

Categories

(Firefox :: Address Bar, defect, P2)

70 Branch
defect
Points:
2

Tracking

()

VERIFIED FIXED
Firefox 71
Iteration:
71.3 - Sept 30 - Oct 13
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox67 --- unaffected
firefox68 --- unaffected
firefox69 --- unaffected
firefox70 + verified
firefox71 --- verified

People

(Reporter: mak, Assigned: dao)

References

(Regression)

Details

(Keywords: regression)

Attachments

(4 files, 1 obsolete file)

In the one-off redesign mail-thread Verdi said "I think we should add a shadow to bottom of the panel", so I'm filing this placeholder bug to better define what he wants.

=> Verdi, please give us some specs.

Flags: needinfo?(mverdi)
Attached image megabar_shadow (obsolete) —

We added a shadow in bug 1575360. I've attached a screenshot of what it looks like. Verdi, are we looking for a different shadow here?

I think verdi was referring to the old design.

No longer blocks: urlbar-update-1
Regressed by: 1551598
See Also: → 1262512
Type: enhancement → defect
Keywords: regression
OS: Unspecified → All
Hardware: Unspecified → All
Version: unspecified → 70 Branch

Tracking this spooky Halloween search box issue for 70. We could still take a shadow patch for beta 12 or 13. Past that, wontfix and then 70 will be released sadly shadowless. (Following up in email with mverdi)

Attachment #9096038 - Attachment is obsolete: true
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Iteration: --- → 71.3 - Sept 30 - Oct 13
Summary: Add a shadow to the bottom of the panel → Add a shadow to the bottom of the legacy (non-megabar) results panel

(In reply to Marco Bonardo [::mak] from comment #0)

=> Verdi, please give us some specs.

box-shadow: 0 5px 18px 0 rgba(0, 0, 0, .2);

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

We added a shadow in bug 1575360. I've attached a screenshot of what it looks like. Verdi, are we looking for a different shadow here?

(In reply to Dão Gottwald [::dao] from comment #2)

I think verdi was referring to the old design.

In the meeting I was referring to the old design but I think we should use this value for the new design also.

Flags: needinfo?(mverdi)
Summary: Add a shadow to the bottom of the legacy (non-megabar) results panel → Update megabar box-shadow and add the same box-shadow to the legacy (non-megabar) results panel
Attachment #9098333 - Attachment description: Bug 1584281 - Add box-shadow to the legacy (non-megabar) results panel. r=mak → Bug 1584281 - Update megabar box-shadow and add the same box-shadow to the legacy (non-megabar) results panel. r=mak
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3cb6b5785edf
Update megabar box-shadow and add the same box-shadow to the legacy (non-megabar) results panel. r=mak
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71
Flags: qe-verify+

Comment on attachment 9098794 [details]
Bug 1584281 - Add box-shadow to the legacy (non-megabar) results panel (beta-only patch). r=mak

Beta/Release Uplift Approval Request

  • User impact if declined: Polish issue
  • 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:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Trivial patch
  • String changes made/needed:
Attachment #9098794 - Flags: approval-mozilla-beta?

Comment on attachment 9098794 [details]
Bug 1584281 - Add box-shadow to the legacy (non-megabar) results panel (beta-only patch). r=mak

CSS tweak for search bar. OK for uplift for beta 13.

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

There are conflicts while trying to graft the patch.
" warning: conflicts while merging browser/themes/shared/urlbar-searchbar.inc.css! "

Flags: needinfo?(dao+bmo)
QA Whiteboard: [qa-triaged]

(In reply to Dão Gottwald [::dao] from comment #9)

Comment on attachment 9098794 [details]
Bug 1584281 - Add box-shadow to the legacy (non-megabar) results panel (beta-only patch). r=mak

  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce:

Hi, Dão can you please add the exact steps or what is the action that needs to be done to be able to verify this fix? Thanks

Steps: Type in the address bar, see if the results panel has a drop shadow.

Flags: needinfo?(dao+bmo)

Thanks, I followed the steps and just to be sure that we are on the same page please see the attached doc and let me know if this is the expected. I'm using a Mac OS X 10.14 with FF Nightly 71.0a1(2019-10-07).

Flags: needinfo?(dao+bmo)
Depends on: 1587027

(In reply to ovidiu boca[:Ovidiu] from comment #15)

Created attachment 9099490 [details]
This is the drop shadow from the results panel?.png

Thanks, I followed the steps and just to be sure that we are on the same page please see the attached doc and let me know if this is the expected. I'm using a Mac OS X 10.14 with FF Nightly 71.0a1(2019-10-07).

No, that's the focus ring. Apparently the megabar drop shadow is broken on Mac, filed bug 1587027 on this.

Flags: needinfo?(dao+bmo)

I see, so this should be verified on Windows and Linux OSes? If yes please attach a spec or a print screen with the expected result. Thanks

Flags: needinfo?(dao+bmo)

You don't need to bother verifying in Nightly. On beta we're going from no shadow at all to having a shadow, so just make sure you see that. It's the same shadow you should see in Nightly on Windows and Linux, if you need a reference.

Flags: needinfo?(dao+bmo)
Attached image shadow.jpg

I tested it on Windows 10 and Ubuntu 18.04 with FF beta 70.b13 and FF Nightly 71.0a1(2019-10-08) and I see this result, please see the attached print screen. Dao, please confirm if this is the expected result for this fix?

Flags: needinfo?(dao+bmo)

Looks good, thanks.

Flags: qe-verify+
Flags: needinfo?(dao+bmo)
Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: