Closed
Bug 1377845
Opened 5 years ago
Closed 5 years ago
Search tooltip is displayed in the wrong position within subdialogs
Categories
(Firefox :: Preferences, defect, P1)
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
(4 files)
[Affected versions]: Nightly 56.0a1 [Affected platforms]: Platforms: Windows 10 x 64, Ubuntu 16.10 and Mac OS X 10.12 [Steps to reproduce]: 1. Go to "about:preferences" and search for "ff" 2. Click on "Choose..." button. 3. Observe the popup. [Expected result]: Tooltip should not be displayed. [Actual result]: Tooltip is displayed in a popup when searching for certain keywords.
Reporter | ||
Updated•5 years ago
|
Blocks: 1357285
status-firefox54:
--- → unaffected
status-firefox55:
--- → unaffected
status-firefox56:
--- → affected
Whiteboard: [photon-preference]
Assignee | ||
Updated•5 years ago
|
Assignee: nobody → rchien
Status: NEW → ASSIGNED
Flags: qe-verify+
Priority: -- → P1
Whiteboard: [photon-preference] → [photon-preference][triage]
Target Milestone: --- → Firefox 56
Assignee | ||
Comment 1•5 years ago
|
||
* Fixed TypeError: frame is undefined when resizing sub-dialog window. * Using vbox rather than hbox or box surrounding menulist element to ensure menulist itself is able to fill available space. * Fixed the margin for xul:label.menu-iconic-highlightable-text. see attachment.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•5 years ago
|
||
Hey Jared, Can you review this patch kindly? Note that this patch has fixed a couple of regressions which mentioned on comment 1. Thanks
Flags: needinfo?(jaws)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 5•5 years ago
|
||
Update patch for fixing root cause of this issue.
Appending a span.search-tooltip element directly on a XUL's row element (part of grid) will cause the layout issue. The search tooltip element will be unable to take out of document flow as expected and then tooltip itself will be placed at the end of row like attachment 8882988 [details].
To fix this issue, we can wrap an extra box element (hbox/vbox/box) around all menulist and tooltip button (for those buttons which are able to open sub-dialogs) to ensure that they are inside a box element.
Updated•5 years ago
|
QA Contact: hani.yacoub
Updated•5 years ago
|
Whiteboard: [photon-preference][triage] → [photon-preference]
Updated•5 years ago
|
Attachment #8883227 -
Flags: review?(jaws) → review?(MattN+bmo)
Comment 6•5 years ago
|
||
_onResize was just changed in bug 1340987 which was reviewed by MattN, so forwarding review to him.
Flags: needinfo?(jaws)
Comment hidden (mozreview-request) |
Reporter | ||
Updated•5 years ago
|
Whiteboard: [photon-preference] → [photon-preference][triage]
Comment 8•5 years ago
|
||
this is under fixing, remove triage tag
Whiteboard: [photon-preference][triage] → [photon-preference]
Comment hidden (mozreview-request) |
Comment 10•5 years ago
|
||
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6) > _onResize was just changed in bug 1340987 which was reviewed by MattN, so > forwarding review to him. …except I think that the _onResize change is fixing a separate bug and should have been a separate commit… I don't know enough about the overlays to understand whether it's really necessary to change natural markup by adding boxes around things… Ricky, please don't combine fixes for multiple very different issues in one commit as it makes it harder to review like now where neither Jared or I can easily review the whole commit. Please split it into one commit for each bullet in comment 1.
Summary: Wrong Tooltip is displayed in a popup when searching for certain keywords → Search tooltip is displayed in the wrong position within subdialogs
Comment 11•5 years ago
|
||
mozreview-review |
Comment on attachment 8883227 [details] Bug 1377845 - Fix wrong tooltip position when menulist and button appear in grid https://reviewboard.mozilla.org/r/154160/#review160288 Please ask scottwu (and I) for review on the subdialogs.js change after splitting.
Attachment #8883227 -
Flags: review?(MattN+bmo)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 14•5 years ago
|
||
Sound great! Patch has been split into two commits for reviewing. Jared, can you review the XUL layout changes in sub-dialogs? Scout, can you take a look for resizing regression in subdialogs.js? It's just one line patch for fixing `TypeError: frame is undefined` when resizing sub-dialog window. Thanks
Comment 15•5 years ago
|
||
mozreview-review |
Comment on attachment 8884684 [details] Bug 1377845 - Fix TypeError: frame is undefined when resizing sub-dialog window https://reviewboard.mozilla.org/r/155550/#review160554 ::: commit-message-21fbd:1 (Diff revision 1) > +Bug 1377845 - Fix TypeError: frame is undefined when resizing sub-dialog window r?scottwu MattN should also take a look. ::: browser/components/preferences/in-content-new/subdialogs.js:331 (Diff revision 1) > })); > this._overlay.style.visibility = "visible"; > this._overlay.style.opacity = ""; // XXX: focus hack continued from _onContentLoaded > > if (this._box.getAttribute("resizable") == "true") { > + this._onResize = this._onResize.bind(this); We should also fix the issue in old preferences: http://searchfox.org/mozilla-central/rev/b7e531410ebc1c7d74a78e86a894e69608db318c/browser/components/preferences/in-content/subdialogs.js#331
Attachment #8884684 -
Flags: review?(scwwu) → review+
Comment hidden (mozreview-request) |
Comment 17•5 years ago
|
||
mozreview-review |
Comment on attachment 8884684 [details] Bug 1377845 - Fix TypeError: frame is undefined when resizing sub-dialog window https://reviewboard.mozilla.org/r/155550/#review160754 Thanks
Attachment #8884684 -
Flags: review?(MattN+bmo) → review+
Comment 18•5 years ago
|
||
mozreview-review |
Comment on attachment 8883227 [details] Bug 1377845 - Fix wrong tooltip position when menulist and button appear in grid https://reviewboard.mozilla.org/r/154160/#review161002 I would rather not have to introduce new elements to wrap existing ones. It seems like this will be a slippery slope. I have put a patch in bug 1379208 which might give us an alternate path forward where we don't need to create new DOM elements.
Attachment #8883227 -
Flags: review?(jaws) → review-
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 23•5 years ago
|
||
mozreview-review |
Comment on attachment 8883227 [details] Bug 1377845 - Fix wrong tooltip position when menulist and button appear in grid https://reviewboard.mozilla.org/r/154160/#review161292 ::: browser/components/preferences/fonts.xul:100 (Diff revision 7) > > <row align="center"> > <hbox align="center" pack="end"> > <label accesskey="&proportional.accesskey;" control="defaultFontType">&proportional.label;</label> > </hbox> > + <hbox> Can you please add a comment next to each one of these boxes that you're introducing stating that these are here to get search tooltips positioned correctly? /* This <hbox> is needed to position search tooltips correctly. */
Attachment #8883227 -
Flags: review?(jaws) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 28•5 years ago
|
||
Pushed by rchien@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/46a7174bf44d Fix wrong tooltip position when menulist and button appear in grid r=jaws https://hg.mozilla.org/integration/autoland/rev/a92d98b28d10 Fix TypeError: frame is undefined when resizing sub-dialog window r=MattN,scottwu
Comment 29•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/46a7174bf44d https://hg.mozilla.org/mozilla-central/rev/a92d98b28d10
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Comment 30•5 years ago
|
||
I can reproduce this issue in Nightly 56.0a1 (2017-07-03) in 64-bit Linux I can verify that this issue is fixed in latest Nightly 56.0a1 Build ID 20170714100217 User Agent Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0
QA Whiteboard: [bugday-20170712]
Comment 31•5 years ago
|
||
I have reproduced this bug with Nightly 56.0a1 (2017-07-03) on Windows 8.1 , 64 Bit ! This bug's fix is Verified with latest Nightly 56.0a1 ! Build ID 20170714030205 User Agent Mozilla/5.0 (Windows NT 6.3; WOW64; rv:56.0) Gecko/20100101 Firefox/56.0 [bugday-20170712]
Comment 32•5 years ago
|
||
As this bug is verified in both linux (comment 30) and windows (comment31), I am marking this bug as verified fixed.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•