Closed Bug 1384566 Opened 3 years ago Closed 3 years ago

Wrong Tooltip position after searching for certain keywords

Categories

(Firefox :: Preferences, defect, P1)

56 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 56
Tracking Status
firefox54 --- unaffected
firefox55 --- unaffected
firefox56 --- verified

People

(Reporter: hani.yacoub, Assigned: rickychien)

References

(Blocks 1 open bug)

Details

(Whiteboard: [photon-preference])

Attachments

(3 files, 4 obsolete files)

Attached image 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.
Blocks: 1357285
Whiteboard: [photon-preference][triage]
Attached image tooltip position 1.png
Flags: qe-verify+
Priority: -- → P2
Target Milestone: --- → Firefox 56
Assignee: nobody → rchien
Status: NEW → ASSIGNED
Priority: P2 → P1
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 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?
Attachment #8890795 - Flags: review?(mconley) → review+
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.
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)
Attachment #8890795 - Attachment is obsolete: true
Attachment #8890795 - Attachment is obsolete: false
Attachment #8891140 - Attachment is obsolete: true
Attachment #8891137 - Attachment is obsolete: true
Attachment #8891137 - Flags: review?(mconley)
Attachment #8891137 - Attachment is obsolete: true
Attachment #8891137 - Flags: review?(mconley)
Attachment #8891141 - Attachment is obsolete: true
Attachment #8891137 - Attachment is obsolete: true
Attachment #8891137 - Flags: review?(mconley)
Attachment #8891144 - Attachment is obsolete: true
Attachment #8891144 - Flags: review?(mconley)
Attachment #8890795 - Attachment is obsolete: true
Comment on attachment 8891137 [details]
Bug 1384566 - Fix incorrect tooltip position by wrapping hbox

https://reviewboard.mozilla.org/r/162322/#review167832
Attachment #8891137 - Flags: review?(mconley) → review+
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)
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/78b337ef1476
Fix incorrect tooltip position by wrapping hbox r=mconley
https://hg.mozilla.org/mozilla-central/rev/78b337ef1476
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Duplicate of this bug: 1385243
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.
Status: RESOLVED → VERIFIED
Whiteboard: [photon-preference][triage] → [photon-preference]
You need to log in before you can comment on or make changes to this bug.