Closed Bug 1393406 Opened 3 years ago Closed 3 years ago

[macOS] [Photon] Regression: The Location Bar Drop Down Result List is 1px too high

Categories

(Firefox :: Theme, defect)

Unspecified
macOS
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 1406353
Firefox 57
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- unaffected
firefox57 --- fixed

People

(Reporter: mehmet.sahin, Unassigned)

References

Details

(Keywords: regression)

Attachments

(3 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3194.0 Safari/537.36

Steps to reproduce:

Latest Nightly from 23 Aug. 2017
macOS 10.12.6

1.) Type something into the Location Bar
2.) Take a look at the position of the Drop Down Result List


Actual results:

The Location Bar Drop Down Result List is 1px too high


Expected results:

The Location Bar Drop Down Result List must be 1px lower.

This is a regression after Bug 1391191 has been fixed now.
Attached video Screencast
Component: Untriaged → Theme
OS: Unspecified → Mac OS X
Blocks: 1391191
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Priority: -- → P4
Whiteboard: [reserve-photon-visual]
Why this is only optional?
Assignee: nobody → dharvey
Status: NEW → ASSIGNED
Iteration: --- → 57.3 - Sep 19
Flags: qe-verify?
Priority: P4 → P1
This is fixed with margin-top: 1px @ http://searchfox.org/mozilla-central/source/toolkit/themes/osx/global/autocomplete.css#41, however it seems only needed on osx and that doesnt seem like a particularly clean fix

Dao is there another way I should go about this? It seems like whatever is placing the panel now needs fixed but I cant tell where that comes from. 

Cheers
Flags: needinfo?(dao+bmo)
As mentioned in bug 1391191, note that the navbar height seems to have increased overall on OSX. I don't currently know if that's intended or if we should fix it, rather than add margin-top to the autocomplete.

https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=a9d372645a32b8d23d44244f351639af9d73b96a&newProject=mozilla-central&newRev=c7570eb46382ee56d081e549b484341f400c864b&filter=osx
(In reply to Johann Hofmann [:johannh] from comment #4)
> As mentioned in bug 1391191, note that the navbar height seems to have
> increased overall on OSX. I don't currently know if that's intended or if we
> should fix it, rather than add margin-top to the autocomplete.
> 
> https://screenshots.mattn.ca/compare/?oldProject=mozilla-
> central&oldRev=a9d372645a32b8d23d44244f351639af9d73b96a&newProject=mozilla-
> central&newRev=c7570eb46382ee56d081e549b484341f400c864b&filter=osx

Hi Johann,

I am the reporter of this bug and from my point of view the +1px (bottom-)height of the navbar is correct, because Bug 1391191 fixed my reported alignment Bug 1391695.

So adding the 1px missing margin-top to the autocomplete would be great.

Thanks in advance.
Ah, thanks for chiming in. I didn't have the context of bug 1391695 (was on PTO). The height adjustment is probably correct then :)
Flags: needinfo?(dao+bmo)
So this.DOMWindowUtils.getBoundsWithoutFlushing(aInput).bottom is returning a .5 value (69.5) I havent been able to figure out why thats possible but ceiling this looks to fix it consistently for me
Comment on attachment 8906659 [details]
Bug 1393406 - Give autocomplete panel extra margin.

https://reviewboard.mozilla.org/r/178374/#review183434

::: browser/base/content/urlbarBindings.xml:1946
(Diff revision 1)
>            }
>  
>            // Position the popup below the navbar.  To get the y-coordinate,
>            // which is an offset from the bottom of the input, subtract the
>            // bottom of the navbar from the buttom of the input.
> -          let yOffset =
> +          let yOffset = Math.ceil(

I think we should probably use Math.round here
Attachment #8906659 - Flags: review?(dao+bmo) → review+
Pushed by dharvey@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/677faa177ebc
Fix OSX autocomplete positioning. r=dao
https://hg.mozilla.org/mozilla-central/rev/677faa177ebc
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Hello Dale Harvey, this report is marked as fixed since yesterday, but I still can reproduce it in 57.0a1 (2017-09-14) (64-Bit). Can you please check it in latest Trunk, too? Thank you in advance.
(In reply to Mehmet from comment #14)
> Hello Dale Harvey, this report is marked as fixed since yesterday, but I
> still can reproduce it in 57.0a1 (2017-09-14) (64-Bit). Can you please check
> it in latest Trunk, too? Thank you in advance.

A friendly ping :-) Can you please take a look at this issue again? The Location Bar Drop Down is still 1px too high, although the fix has been landed and the report is marked as fixed. Thank you in advance.
Flags: qe-verify? → qe-verify+
QA Contact: ovidiu.boca
Hi Dale, can you please take a look at comment 14? Thanks
Flags: needinfo?(dharvey)
I tested on Mac OS X 10.10 with FF Nightly 57.0a1(2017-09-19) and I can still reproduce this.
Yup I can see this happening again, will reopen and look again
Status: RESOLVED → REOPENED
Flags: needinfo?(dharvey)
Resolution: FIXED → ---
Status: REOPENED → ASSIGNED
Iteration: 57.3 - Sep 19 → ---
So looking again, not seeing any .5 values this time round. The awesomebar is positioned by taking nav-bar.bottom - aInput.bottom, this doesnt count the bottom border as thats in #navigator-toolbox and the awesome bar has its own top border so is supposed to overlay.

On osx we have nav-bar.bottom = 71 and aInput.bottom = 67 with a 4px offset, On linux with have a nav-bar.bottom = 75 and aInput.bottom = 69, the 6px puts it as with osx right at the line of the border, linux shows this correctly but osx only gives a 3px offset, the url bar isnt moving or losing the border, the panel just looks to be in the wrong place
The gaps in that are the 4px (osx) and 6px (linux) measured, they both end in the same place so looks like |this.openPopup| has a problem?
Dao, should we go for the margin-top:1px on osx to fix this or look further into the root cause? if so some guidance onto the root cause here would be good as I am a little stuck, thanks
Flags: needinfo?(dao+bmo)
(In reply to Dale Harvey (:daleharvey) from comment #21)
> Dao, should we go for the margin-top:1px on osx to fix this

Works for me as long as you document it properly.
Flags: needinfo?(dao+bmo)
Closing as a dupe as reviewboard wont let me send follow up patches to this bug
Status: ASSIGNED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1406353
Assignee: dharvey → nobody
Flags: qe-verify+
Priority: P1 → --
QA Contact: ovidiu.boca
Whiteboard: [reserve-photon-visual]
You need to log in before you can comment on or make changes to this bug.