Wrong Tooltip position after searching for certain keywords

VERIFIED FIXED in Firefox 56

Status

()

Firefox
Preferences
P1
normal
VERIFIED FIXED
29 days ago
24 days ago

People

(Reporter: Hani Yacoub, Assigned: rickychien)

Tracking

(Blocks: 1 bug)

56 Branch
Firefox 56
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox54 unaffected, firefox55 unaffected, firefox56 verified)

Details

(Whiteboard: [photon-preference][triage])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments, 4 obsolete attachments)

(Reporter)

Description

29 days ago
Created attachment 8890348 [details]
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.
(Reporter)

Updated

29 days ago
Blocks: 1357285
status-firefox54: --- → unaffected
status-firefox55: --- → unaffected
status-firefox56: --- → affected
Whiteboard: [photon-preference][triage]
(Reporter)

Comment 1

29 days ago
Created attachment 8890349 [details]
tooltip position 1.png
(Reporter)

Updated

29 days ago
Flags: qe-verify+
(Assignee)

Updated

28 days ago
Priority: -- → P2
Target Milestone: --- → Firefox 56
(Assignee)

Updated

28 days ago
Assignee: nobody → rchien
Status: NEW → ASSIGNED
Priority: P2 → P1
Comment hidden (mozreview-request)
(Assignee)

Comment 3

28 days ago
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

28 days ago
mozreview-review
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+
(Assignee)

Comment 5

28 days ago
mozreview-review-reply
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 hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 10

27 days ago
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 hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

27 days ago
Attachment #8890795 - Attachment is obsolete: true
(Assignee)

Updated

27 days ago
Attachment #8890795 - Attachment is obsolete: false
Comment hidden (mozreview-request)
(Assignee)

Updated

27 days ago
Attachment #8891140 - Attachment is obsolete: true
(Assignee)

Updated

27 days ago
Attachment #8891137 - Attachment is obsolete: true
Attachment #8891137 - Flags: review?(mconley)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

27 days ago
Attachment #8891137 - Attachment is obsolete: true
Attachment #8891137 - Flags: review?(mconley)
(Assignee)

Updated

27 days ago
Attachment #8891141 - Attachment is obsolete: true
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

27 days ago
Attachment #8891137 - Attachment is obsolete: true
Attachment #8891137 - Flags: review?(mconley)
(Assignee)

Updated

27 days ago
Attachment #8891144 - Attachment is obsolete: true
Attachment #8891144 - Flags: review?(mconley)
Comment hidden (mozreview-request)
(Assignee)

Updated

27 days ago
Attachment #8890795 - Attachment is obsolete: true

Comment 20

27 days ago
mozreview-review
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+
Comment hidden (mozreview-request)

Comment 22

27 days ago
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 hidden (mozreview-request)

Comment 24

26 days ago
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/78b337ef1476
Fix incorrect tooltip position by wrapping hbox r=mconley

Comment 25

25 days ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/78b337ef1476
Status: ASSIGNED → RESOLVED
Last Resolved: 25 days ago
status-firefox56: affected → fixed
Resolution: --- → FIXED
(Assignee)

Updated

24 days ago
Duplicate of this bug: 1385243
(Reporter)

Comment 27

24 days ago
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
status-firefox56: fixed → verified
You need to log in before you can comment on or make changes to this bug.