Does not take in consideration zoom CSS property

[osasseville]

The popper does no take in consideration the zoom css property of it's parents.

If you put a zoom on the parent and resize the window, you can clearly see that the popper will behave as it was not zoomed.

https://codepen.io/osasseville/pen/bYLmEj

Steps to reproduce the problem

1. Open the code pen
2. Resize the height of the window
3. Note that the greenbox move "up" then there's still space.
4. Remove the zoom css property from the .container
5. Note that the greenbox move "up" only when there's no more space.

Browser versions

Chrome Version 62.0.3202.94
Microsoft Edge 41.16299.15.0

[wclr]

Not so difficult.

[FezVrasta]

@whitecolor feel free to send a PR if you have ideas on how to fix this!

[lucky7id]

For my own understanding, why is this marked priority low? Currently for anyone utilizing this library through the large number of UI libs this is causing chrome to utilize 100% CPU and cause crashes for anyone not using 100% zoom. It would be great to see a higher prioritization

[FezVrasta]

Because nobody before you asked to prioritize it, a bunch of stuff doesn't work when the browser is zoomed... And usually there's not much we can do to fix it.

[atomiks]

zoom should never be used anyway, use transform: scale. It's not a standard property and behaves differently between browsers.

[wclr]

transform: scale or any other causes positioning problems anyway.

[lucky7id]

@atomiks I'm referring to the browser zoom. This original issue mentions the CSS zoom property, but you will see that this issue is referenced in a different library specifically mentioning browser zoom. I am happy to make a separate issue as the scope may be different. But really, the issue is that popper can enter into an infinite loop, utilizing 100% cpu. I understand that of course the tooltips themselves may not work properly under zoom (css or browser), but at the very least preventing the loop from occurring should be top priority

edit: cc @FezVrasta

[atomiks]

@lucky7id I think it varies between browsers, as Chrome on Mac handles browser zoom fine regarding popper positioning (when I zoom the CodePen window, for example)

[lucky7id]

@atomiks Using chrome on mac myself, can repro 100% of the time. Interestingly I am only able to repro at certain zoom levels, and specifically only when zoomed out (so far as I have been able to repro)

[FezVrasta]

Please open a separated issue then.

[fentie]

I can confirm this as well. Chrome Mac, using browser zoom out to about 80%, locks up and crashes. If there's any other info/logs/etc I can provide, let me know.

[lucky7id]

I've opened a new issue here: #578
if anyone can contribute a good pen that would be awesome!

[ryanswanson]

We recently addressed this issue for our own library and now only have it when using Popper.js in a few components. To contribute to a fix here, I have created a code pen that includes two utility methods that address the issue and a simple example (target practice) to illustrate the issue and fix.

https://codepen.io/ryanswanson/pen/XveKww

In places where you would typically use element.getBoundingClientRect(), you can instead use getZoomAdjustedBoundingClientRect(element). This utility method uses getBoundingClientRect() internally and scales its results by a calculated 'ancestral zoom' factor, which is the computed zoom scale for all ancestors multiplied together (getAncestralZoom method).

Side note: Firefox doesn't support zoom CSS, so this fix isn't needed, but to handle browsers that don't support zoom, getAncestralZoom applies a default of 1.

I have not evaluated the Popper codebase to see where this fix should be applied (i.e. to create a PR), since I would first want to see if the library maintainers consider this an adequate resolution. If so, please let me know if I should create a PR to apply a fix. We would love to have this resolved soon for an upcoming release.

[FezVrasta]

Thanks for the reply. From what I see, your solution needs to traverse the whole DOM any time you need to measure the element. I wonder how will it affect performance?

[ryanswanson]

Glad to pitch in!

tl;dr - It seems to perform quite well from my limited testing.

The getAncestralZoom algorithm does call window.getComputedStyle() for each of the target node's direct ancestor elements (within a document - doesn't go outside iframes), the cost of which may matter depending on how it is used (how frequently called).

We primarily have used this utility for initial positioning of an overlay element; however, for 'live' positioning (keeping an overlay position synchronized to match the location of a moving element due to scroll, resize, etc.), the successive calls to reposition may need to be debounced/throttled for high-volume triggers such as scrolling or mouse move. The question is how frequently can it refresh and still perform well.

When I originally profiled this solution using Chrome DevTools Performance Profiling, I became rather less concerned about the incremental cost making a measurable difference. I couldn't find my previous tests, so I ran a couple against the code pen I've shared above. The attached zip includes Profiles that you can load in Chrome Dev Tools Performance tab.

To isolate only successive calls to getAncestralZoom (and thus getComputedStyle), I created a fork of my code pen here: https://codepen.io/ryanswanson/pen/QeQvzR. First, I altered it to render in one state: zoomed out with the fix applied. I also dumped a large amount of CSS into the example (code pen's CSS here: https://static.codepen.io/assets/editor/editor-87a3eb18763d3b6732a08729d3d71cbc6396dc1d9b4f7e709eaeec79064c78da.css).

Each test varies timeout and DOM depth. The timeout varies between the original 3000ms and 10ms (which would illustrate live updating @100fps). The DOM structure varies from the original, shallow example to one that adds 10 outer divs so it requires either 4 or 14 (10 extra divs) recursive calculations with getComputedStyle().

A differential between these profiles should identify the degree to which depth affects overall performance and a pretty good indication of absolute performance. I would expect the the cost of style recalc to track with the amount of CSS, so your mileage may vary somewhat.

Interval  Depth  Scripting Time  System Time  Idle Time   Total Time  Non-Idle %
3000      4      78.0ms          37.1ms       10,183.4ms  10,300ms    1.13%
3000      14     79.4ms          5.0ms        10,022.8ms  10,110ms    0.86%
10        4      378.5ms         47.0ms       9,813.4ms   10,240ms    4.17%
10        14     275.4ms         57.3ms       9,639.8ms   9,970ms     3.31%

The target node's depth seems to have a negligible effect. Perhaps using getComputedStyle once may update the 'live' style object after which subsequent calls are fast during the same execution.

Overall performance seems good. The Bottom-Up total time for ~1000 calls (every 10ms for 10s) to getAncestralZoom was 24.1ms, or 0.0241ms per call (10ms/14 profile). This was a fraction of the other example code running at the same interval with total script time at ~300ms for ~1000 intervals or ~0.3ms per interval.

Cheers,
Ryan

p.s. I noticed that the 'zoom' property is not listed in Paul Irish's list of properties that trigger force layout, which may be our saving grace here. https://gist.github.com/paulirish/5d52fb081b3570c81e3a

getAncestralZoomProfiling.zip

[atomiks]

@ryanswanson what are the reasons for using zoom over transform: scale?

[ryanswanson]

@atomiks Fair question. I've found they actually work quite differently and that transform: scale() can only be a suitable replacement for some use cases, particularly when the scaled content is not interactive.

Transform: scale() affects only the rendered appearance of the targeted content and is applicable for image scaling, drag/drop effects, etc.

Zoom will cause dimensions of the targeted content to change resulting in a reflow within the available space.

Our use case: We manage a framework that provides windowing and workspace layout features that includes the ability to zoom the content (applications) within a region of the layout. All features have to function properly at all zoom levels. I do not believe this is achievable with transform: scale() - we have tried it before in an attempt to make this feature available in Firefox, to no avail.

Hopefully the following image will illustrate what I'm trying to describe. I just updated the sidebar at the top of this page to compare how it looks originally, with zoom and with transform: scale().

[FezVrasta]

@ryanswanson I hope you understand me if I'm being a bit reluctant to add support for this, there are a ton of things that could go wrong, and a very limited set of users that would benefit from it.

I would say I'd be willing to merge a PR if it doesn't add too much complexity to the codebase and ships with a good set of functional tests, but it will really have to convince me completely to get merged.

[wclr]

@FezVrasta what I would think of is generic extensible API for custom positioning calculations -)

[FezVrasta]

If you have ideas to make this possible feel free to open an explorative PR targeting the next branch. I don't expect a working solution, but just some code to show what you have in mind.

[FezVrasta]

So, in v2 the popper offsets calculation is in a dedicated modifier, that means anyone can extend or completely replace it to address their own use cases when they are not covered by the default one.

I'll mark this as "Fixed in v2"

[ryanswanson]

@FezVrasta Thank you for addressing this. We have been running a local fork in the meantime and I'll look forward to getting back on the main line.

Will send feedback once we upgrade. Cheers!

[f0rmat1k]

seems like it wasn't fixed. Maybe should reopen?

[pavanmajji03]

I think this issue should be reopened as chromium project has standardized the css zoom from version 128, and its rolled out.
ref: w3c/csswg-drafts#9699
Comment: https://issues.chromium.org/issues/365913984#comment6
Issue raised: https://issues.chromium.org/issues/365913984

So as per this standardization top and left positions to be corrected which are being used from what are being returned by bounding rect methods in js.

A detailed doc outlining the affecting methods is here: https://docs.google.com/document/d/1AcnDShjT-kEuRaMchZPm5uaIgNZ4OiYtM4JI9qiV8Po/edit?tab=t.0

[atomiks]

@pavanmajji03 New issue for it: #3032