Positioning the search tooltips should not need to flush layout

RESOLVED DUPLICATE of bug 1363730

Status

()

P1
normal
RESOLVED DUPLICATE of bug 1363730
a year ago
a year ago

People

(Reporter: jaws, Unassigned)

Tracking

(Blocks: 1 bug)

unspecified
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Bug 1351867 implemented showing the search tooltips but it uses getBoundingClientRect to figure out where to absolutely position the tooltip.

http://searchfox.org/mozilla-central/rev/f1472e5b57fea8d4a7bbdd81c44e46958fe3c1ce/browser/components/preferences/in-content-new/findInPage.js#434

We remove this code and instead just add the following CSS styles to .search-tooltip:

  top: -55%;
  left: 0;
  right: 0;
  width: -moz-fit-content;
  margin-left: auto;
  margin-right: auto;
Actually this positions the tooltip when searching for "container" in the center when it is expected to be above the button on the right side. I'll need to figure something else out, and I assume there are cases where top:0; isn't the right thing either.

If we could just use ::before on the anchor element instead of creating a new adjacent node that has to be positioned absolutely relative to their common ancestor.
I've hit an impasse working on this and it comes back to bug 1363730 so I'm going to close this and dupe it to bug 1363730.
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1363730
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Comment hidden (mozreview-request)
Created attachment 8885079 [details]
wrong-tooltip.png

Tooltip position seems wrong after applying the patch.
Status: REOPENED → ASSIGNED
Flags: qe-verify-
Priority: -- → P1
Whiteboard: [photon-preference][triage]

Comment 5

a year ago
mozreview-review
Comment on attachment 8885024 [details]
Bug 1379208 - Position the tooltips using only one layout flush and no modifications to the surrounding DOM of the tooltip nodes.

https://reviewboard.mozilla.org/r/155850/#review161036

Jared,

Thanks for helping work on this. Without pseudo element and postion: abolution support for XUL element, the current workaround causes so many regressions and incorrect tooltip position issues. I'd like this patch and reduce layout flush as much as possible.

Please see my attachment. I saw that patch broke tooltip position.
Attachment #8885024 - Flags: review?(rchien) → review-
After playing with it for a while, it seems like still buggy such as hover mouse on tooltip is unable to see the opacity effect unless you hover on button itself.

My conclusion is that with `position: fixed` solution treating the pseudo element as a part of the anchor element, so tooltip will not be an independent element and support the hover effect as expected.
Well it was worth a shot and I learned something new about transform and position:fixed :)
Assignee: jaws → nobody
Status: ASSIGNED → RESOLVED
Last Resolved: a year agoa year ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1363730

Comment 8

a year ago
mozreview-review
Comment on attachment 8885024 [details]
Bug 1379208 - Position the tooltips using only one layout flush and no modifications to the surrounding DOM of the tooltip nodes.

https://reviewboard.mozilla.org/r/155850/#review161294
Attachment #8885024 - Flags: review?(mconley)
remove whiteboard tag due to its DUPLICATE
Whiteboard: [photon-preference][triage]
You need to log in before you can comment on or make changes to this bug.