1384566 - Wrong Tooltip position after searching for certain keywords

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P1)

Description

[Hani Yacoub, Desktop QA]

Attachment: tooltip position 2.png

[Affected versions]: 
Nightly 56.0a1

[Affected platforms]:
Platforms: Windows 10 x 64, Ubuntu 16.04 and Mac OS X 10.12

[Steps to reproduce]:
1. Launch Firefox and go to "about:preferences".
2. Start typing to search for something (e.g yahoo or co).

[Expected result]:
The position of the Tooltip should be correct.

[Actual result]:
Wrong Tooltip position after searching for certain keywords.

Comment 1

[Hani Yacoub, Desktop QA]

Attachment: tooltip position 1.png

Comment 2

[Ricky Chien [:rickychien] (inactive)]

Attachment: WIP

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

Comment 3

[Ricky Chien [:rickychien] (inactive)]

Mike,

The issue was caused by Reorg v2 (bug 1365133) since we clean up some unused wrapping hbox accidentally. Those additional wrapping hbox should add comment to inform developer do not remove it.

I've verified all preferences XUL files and wrapped all tooltip anchors (such as menulist & sub-dialog button) with a hbox in order to correct the tooltip position.

On the other hand, the previous window.resize workaround (Bug 1374230) will be unless and removed since the current hbox trick can get rid of these resize issue easily.

Please review it kindly. Thanks

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8890795 [details]
WIP

https://reviewboard.mozilla.org/r/161996/#review167296

Thanks! Just two notes.

Also, I strongly encourage adding automated testing for tooltip positions.

::: browser/components/preferences/in-content-new/main.xul:435
(Diff revision 1)
>                  onsyncfrompreference="return gPrivacyPane.readBrowserContainersCheckbox();"/>
>        <label id="browserContainersLearnMore" class="learnMore text-link">
>          &browserContainersLearnMore.label;
>        </label>
>        <spacer flex="1"/>
> +      <!-- Don't remove - wrapping a nsBoxFrame (box/hbox/vbox) for correcting the search tooltip position -->

We could probably make this (and the other messages) a bit clearer - perhaps:

<!-- Please don't remove the wrapping hbox/vbox/box for these elements. It's used to properly compute the search tooltip position. -->

::: browser/components/preferences/in-content-new/search.xul:33
(Diff revision 1)
> +      <hbox>
> +        <!-- Don't remove - wrapping a nsBoxFrame (box/hbox/vbox) for correcting the search tooltip position -->
> +        <hbox>

We need two nested hbox's here?

Comment 5

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8890795 [details]
WIP

https://reviewboard.mozilla.org/r/161996/#review167296

> We need two nested hbox's here?

If there is only one hbox which will stretch and fit the parent element, so that the tooltip will position at the center of the hbox itself but not at the center of the anchor element (<menulist id="defaultEngine">). To fix the issue, we need to add an extra hbox inside outer hbox to ensure that the inner hbox will not fill the entire row.

Comment 6

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8890795 [details]
WIP

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

Comment 7

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8890795 [details]
WIP

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

Comment 8

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8890795 [details]
WIP

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

Comment 9

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8890795 [details]
WIP

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

Comment 10

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 00696eed6cf4 -d 084696d19f1a: rebasing 410157:00696eed6cf4 "Bug 1384566 - Fix incorrect tooltip position by wrapping hbox r=mconley" (tip)
merging browser/components/preferences/in-content-new/privacy.xul
warning: conflicts while merging browser/components/preferences/in-content-new/privacy.xul! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 11

[Ricky Chien [:rickychien] (inactive)]

Attachment: Bug 1384566 - Fix incorrect tooltip position by wrapping hbox

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

Comment 12

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8890795 [details]
WIP

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/161996/diff/5-6/

Comment 13

[Ricky Chien [:rickychien] (inactive)]

Attachment: WIP

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

Comment 14

[Ricky Chien [:rickychien] (inactive)]

Attachment: WIP

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

Comment 15

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8891137 [details]
Bug 1384566 - Fix incorrect tooltip position by wrapping hbox

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

Comment 16

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8891137 [details]
Bug 1384566 - Fix incorrect tooltip position by wrapping hbox

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

Comment 17

[Ricky Chien [:rickychien] (inactive)]

Attachment: Bug 1384566 - Fix incorrect tooltip position by wrapping hbox

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

Comment 18

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8891137 [details]
Bug 1384566 - Fix incorrect tooltip position by wrapping hbox

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

Comment 19

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8891137 [details]
Bug 1384566 - Fix incorrect tooltip position by wrapping hbox

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

Comment 20

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8891137 [details]
Bug 1384566 - Fix incorrect tooltip position by wrapping hbox

https://reviewboard.mozilla.org/r/162322/#review167832

Comment 21

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8891137 [details]
Bug 1384566 - Fix incorrect tooltip position by wrapping hbox

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/162322/diff/5-6/

Comment 22

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 20fd9a5f1261 -d 039ee2964fb9: rebasing 410294:20fd9a5f1261 "Bug 1384566 - Fix incorrect tooltip position by wrapping hbox r=mconley" (tip)
merging browser/components/preferences/in-content-new/privacy.xul
merging browser/components/preferences/in-content-new/search.xul
warning: conflicts while merging browser/components/preferences/in-content-new/search.xul! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 23

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8891137 [details]
Bug 1384566 - Fix incorrect tooltip position by wrapping hbox

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/162322/diff/6-7/

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/78b337ef1476
Fix incorrect tooltip position by wrapping hbox r=mconley

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/78b337ef1476

Comment 27

[Hani Yacoub, Desktop QA]

Build ID: 20170730100307
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0

Verified as fixed on Firefox Nightly 56.0a1 on Windows 10 x 64, Mac OS X 10.12 and Ubuntu 16.04 x64.