1379208 - Positioning the search tooltips should not need to flush layout

Status: RESOLVED DUPLICATE of bug 1363730

(Firefox :: Settings UI, enhancement, P1)

Description

[Jared Wein [:jaws] (please needinfo? me)]

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;

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

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.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

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.

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1379208 - Position the tooltips using only one layout flush and no modifications to the surrounding DOM of the tooltip nodes.

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

Comment 4

[Ricky Chien [:rickychien] (inactive)]

Attachment: wrong-tooltip.png

Tooltip position seems wrong after applying the patch.

Comment 5

[Ricky Chien [:rickychien] (inactive)]

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.

Comment 6

[Ricky Chien [:rickychien] (inactive)]

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.

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

Well it was worth a shot and I learned something new about transform and position:fixed :)

Comment 8

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

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

Comment 9

[Francis Lee [:frlee]]

remove whiteboard tag due to its DUPLICATE