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

Status: RESOLVED DUPLICATE of bug 1406353

(Firefox :: Theme, defect)

Description

[Mehmet]

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.

Comment 1

[Mehmet]

Attachment: Screencast

Comment 2

[Mehmet]

Why this is only optional?

Comment 3

[Dale Harvey (:daleharvey)]

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

Comment 4

[Johann Hofmann [:johannh]]

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

Comment 5

[Mehmet]

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

Comment 6

[Johann Hofmann [:johannh]]

Ah, thanks for chiming in. I didn't have the context of bug 1391695 (was on PTO). The height adjustment is probably correct then :)

Comment 7

[Dale Harvey (:daleharvey)]

Attachment: Bug 1393406 - Give autocomplete panel extra margin.

Review commit: https://reviewboard.mozilla.org/r/178374/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/178374/

Comment 8

[Dale Harvey (:daleharvey)]

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 9

[Dão Gottwald [::dao]]

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

Comment 10

[Dale Harvey (:daleharvey)]

Comment on attachment 8906659 [details]
Bug 1393406 - Give autocomplete panel extra margin.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/178374/diff/1-2/

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/677faa177ebc
Fix OSX autocomplete positioning. r=dao

Comment 12

[Dale Harvey (:daleharvey)]

Gave this a try run and looks fine @ https://treeherder.mozilla.org/#/jobs?repo=try&revision=e074e6d98bb56e9a842a7f3a77ce775c3eeed093&selectedJob=130310885, changed to Math.round

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/677faa177ebc

Comment 14

[Mehmet]

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.

Comment 15

[Mehmet]

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

Comment 16

[ovidiu boca[:Ovidiu]]

Hi Dale, can you please take a look at comment 14? Thanks

Comment 17

[ovidiu boca[:Ovidiu]]

I tested on Mac OS X 10.10 with FF Nightly 57.0a1(2017-09-19) and I can still reproduce this.

Comment 18

[Dale Harvey (:daleharvey)]

Yup I can see this happening again, will reopen and look again

Comment 19

[Dale Harvey (:daleharvey)]

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

Comment 20

[Dale Harvey (:daleharvey)]

Attachment: Screen Shot 2017-09-26 at 13.54.12.png

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?

Comment 21

[Dale Harvey (:daleharvey)]

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

Comment 22

[Dão Gottwald [::dao]]

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

Comment 23

[Dale Harvey (:daleharvey)]

Comment on attachment 8906659 [details]
Bug 1393406 - Give autocomplete panel extra margin.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/178374/diff/2-3/

Comment 24

[Dale Harvey (:daleharvey)]

Comment on attachment 8906659 [details]
Bug 1393406 - Give autocomplete panel extra margin.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/178374/diff/3-4/

Comment 25

[Dale Harvey (:daleharvey)]

Closing as a dupe as reviewboard wont let me send follow up patches to this bug