Skip to content
This repository has been archived by the owner on Feb 16, 2021. It is now read-only.

Position of Flash movie incorrect when CSS zoom is used #149

Open
myitcv opened this issue May 10, 2013 · 37 comments · Fixed by #187
Open

Position of Flash movie incorrect when CSS zoom is used #149

myitcv opened this issue May 10, 2013 · 37 comments · Fixed by #187

Comments

@myitcv
Copy link

myitcv commented May 10, 2013

When using ZeroClipboard with reveal.js problems crop up because reveal.js uses CSS zoom.

This means that the calculation of the position of the glued element (via _getDOMObjectPosition) is incorrect.

The following screenshot shows the problem visually. The red box is the repositioned Flash Movie object, even though the cursor is hovering over the button ("Copy to Clipboard...") to which it is 'glued'

zoom_problem

@JamesMGreene
Copy link
Member

Just a quick note to say that PR #114 may resolve this. Planning to merge it within the next few weeks.

@myitcv
Copy link
Author

myitcv commented May 10, 2013

@JamesMGreene - it does indeed... with one small extra change. If obj.getBoundingClientRect is defined, info should also take its height and width from it:

if (typeof obj.getBoundingClientRect !== "undefined") {
  var rect = obj.getBoundingClientRect();
  var pageXOffset = window.pageXOffset || document.documentElement.scrollLeft || 0;
  var pageYOffset = window.pageYOffset || document.documentElement.scrollTop || 0;
  var leftBorderWidth = document.documentElement.clientLeft || 0;
  var topBorderWidth = document.documentElement.clientTop || 0;
  info.width = rect.width;
  info.height = rect.height;
  info.left = rect.left + pageXOffset - leftBorderWidth;
  info.top = rect.top + pageYOffset - topBorderWidth;
  return info;
}

Which gives the following updated screenshot:

image

Thanks for the quick response.

@JamesMGreene
Copy link
Member

@myitcv: Ah, interesting. Good catch, I'll try to work that in while merging the PR.

@myitcv
Copy link
Author

myitcv commented May 13, 2013

I spoke too soon. Seems like this will not work in Chrome until this bug gets fixed

@myitcv
Copy link
Author

myitcv commented May 13, 2013

@JamesMGreene - sorry, should have messaged you directly with my last comment. Just to make sure you saw it (regarding the bug in webkit)

@JamesMGreene
Copy link
Member

@myitcv No worries, I'm a repo collaborator so I get notifications for all the messages. :)

@JamesMGreene
Copy link
Member

@myitcv: An interesting note that, while researching this a bit more, it seems like people greatly discourage the use of the zoom rule as it is an "oldIE" hack that a few other browser just happened to adopt. Not sure if you are supporting those old IE versions, too? If not, then you can utilize the CSS3 transform rule for scaling instead (in IE10+, and every other browser).

@myitcv
Copy link
Author

myitcv commented May 27, 2013

You are right; my original post was lazy. I don't know the inner workings of reveal.js but I believe it does rely on CSS3 transform as opposed to the ancient zoom concept introduced by IE. My lazy language, nothing else.

@JamesMGreene
Copy link
Member

@myitcv: Any chance you could try out the latest JS and SWF from the master branch (not tagged/released yet)? I think the PR I just merged will fix your zoom issues but I'm not 100% positive.

I would really appreciate it if you could confirm. If not, please reopen this issue. Thanks!

@JamesMGreene
Copy link
Member

Use the latest beta version: https://github.com/zeroclipboard/ZeroClipboard/releases/tag/v1.2.0-beta.3

@myitcv Let me know if this fixes your issues, thanks!

@myitcv
Copy link
Author

myitcv commented Jul 13, 2013

@JamesMGreene - sorry for the delay. Looks good in Firefox, but not Chrome. Will investigate and see if I can't work out the issue.

Is there some way we can write tests for this sort of thing? A la jsfiddle, but in an automated fashion?

@JamesMGreene
Copy link
Member

There are definitely ways but it takes a bit of time: e.g. setting up a BrowserStack account and hooking up some test runner infrastructure.

A short-term improvement is to switch our tests over from nodeunit to browser tests (e.g. QUnit) that run via PhantomJS to allow us to conduct real positioning tests. However, PhantomJS only represents one potential slice of WebKit browsers, so that doesn't cover us in Firefox or IE, and is not always in 100% parity with Safari or Chrome; thus limiting its benefit when testing areas that are not compatible cross-browser.

@JamesMGreene JamesMGreene reopened this Jul 13, 2013
@myitcv
Copy link
Author

myitcv commented Aug 13, 2013

I think I've tracked down the problem (at least my problem when using ZeroClipboard with reveal.js). The key to the problem is this line in reveal.js

reveal.js uses zoom in Chrome "since Chrome blurs scaled content"

This jsFiddle when viewed in Chrome (poorly) highlights the issue.

So I think this means that _getZoomFactor needs to do something similar to the following in case the zoom css property is set (because this bug means that getBoundingClientRect will return an incorrect value in Chrome):

while(obj)
{
  zoom = zoom * _getStyle(obj,'zoom');
  obj = obj.offsetParent;
}

All of this somehow needs to take into account that zoom as you say is deprecated... but very much still in use.

@H1D
Copy link

H1D commented Aug 15, 2013

Just got similar bug but when using transform: translate on glued element.
Using getBoundingClientRect definitely a good idea!

//spent 4 hours digging this =((

@JamesMGreene
Copy link
Member

@H1D: Did you try the latest beta version? https://github.com/zeroclipboard/ZeroClipboard/releases/tag/v1.2.0-beta.3

If it doesn't already resolve your issue, please file it as a new issue as it is not the same as this one despite some similarity.

@JamesMGreene
Copy link
Member

@myitcv: Here's a JSFiddle of what I've got it boiled down to: http://jsfiddle.net/JamesMGreene/cDqXE/

Not pretty but it passes your test. :)

@JamesMGreene
Copy link
Member

I do suspect that the top and left values may get off if there are additional zoom-ed elements in the ancestry, though. :-\

If you want to help out, I'd love to have some QUnit tests written for our positioning logic. QUnit coming soon to a ZeroClipboard repo near you... #216. 😉

@JamesMGreene
Copy link
Member

For the record, jQuery also reports the incorrect height (30) for that element using height(), innerHeight(), and outerHeight().

@H1D
Copy link

H1D commented Aug 15, 2013

@JamesMGreene works fine with 1.2.0-beta.3 !
Also it fixed #217 for me (on Ubuntu)
Thx. sry for flooding

@JamesMGreene
Copy link
Member

You know... positioning and sizing would probably be a lot easier if we could just put ZC into the glued element's parent without getting nailed by CSS issues (e.g. adding a background-color to the nth-child, etc). I suppose we could try to counteract all of those by styling our object element with a bunch of !important notations, e.g. background-color: transparent !important;.

Any thoughts on this @jonrohan?

@myitcv
Copy link
Author

myitcv commented Aug 16, 2013

@JamesMGreene re your comment on jQuery, I'm not surprised. Probably caused by the same underlying bug

@JamesMGreene
Copy link
Member

@myitcv: Sounds like it's fixed in the dev branches: https://code.google.com/p/chromium/issues/detail?id=265252 \o/

@JamesMGreene
Copy link
Member

Although, it looks like those bugs are all about page zoom and not CSS zoom.

@myitcv
Copy link
Author

myitcv commented Aug 16, 2013

Hmm, in which case I think I've linked to the wrong bug. Whichever of the bugs alluded to in that chain which causes getBoundingClientRect to be wrong in Chrome is the one we're interested in.

@jonrohan
Copy link
Contributor

ZC into the glued element's parent without getting nailed by CSS issues

This would make ZC unusable in the GitHub codebase. We have multiple glued elements on the page, having multiple ZC elements eats memory and moving the ZC element in and out of different html causing browser redrawing.

It's a non-starter.

@JamesMGreene
Copy link
Member

That was always my thoughts before as well... had a moment of weakness, I guess, as this zoom thing is pretty frustrating.

I also just remembered that you could still get nailed with CSS issues that you couldn't handle that way anyway, e.g. if the glued element had a style applied by nth-child and the ZC object would likely not meet those same nth-child conditions.

@JamesMGreene
Copy link
Member

@JamesMGreene
Copy link
Member

TODO: See if we can feature test for how zoom affects height and width. If not, then we cannot correctly calculate the height and width cross-browser when CSS zoom is in play. 😕

@myitcv
Copy link
Author

myitcv commented Nov 10, 2013

@JamesMGreene - just a link comment, referencing my comment that refers to the Gist where I made CSS zoom work:

https://gist.github.com/myitcv/7396290

@JamesMGreene
Copy link
Member

Right. As mentioned above, we need to figure out a way to feature test (per browser) if the zoom CSS property is supported and, more importantly, how it affects height and width calculations.

@JamesMGreene
Copy link
Member

The first (support for zoom) should be easily testable with the following snippet:

var isSupported = "zoom" in document.body.style;

Firefox and pre-WebKit/Blink Opera will return false. IE, Safari, Chrome, and post-WebKit/Blink Opera will return true.

@JamesMGreene
Copy link
Member

I updated my fiddle to include getBoundingClientRect in the attempted feature tests. Still fails as suspected.

http://jsfiddle.net/JamesMGreene/2Rn3F/

Looks like we may have to resort to hacky browser detection in order to make this work. 👎

@ghost ghost assigned JamesMGreene Dec 24, 2013
@JamesMGreene
Copy link
Member

Doing some more research on this today... unexpectedly found a scenario where Chrome does shrink the width after zoom. WTF!

http://jsfiddle.net/JamesMGreene/8BSVq/

@JamesMGreene
Copy link
Member

This issue makes my brain hurt. Removing from the final 1.x milestone. 😕

@JamesMGreene JamesMGreene added this to the Release 2.x (> 2.0) milestone Mar 21, 2014
@JamesMGreene JamesMGreene modified the milestones: Release 2.x (>= 2.1), Release 2.0.x Infrastructure Enhancements May 24, 2014
@JamesMGreene
Copy link
Member

Related: #495 ("Create positioning unit tests")

@SeanZicari
Copy link

SeanZicari commented Jun 21, 2016

I've got some HTML that uses CSS transform rules, and the same positioning problem exists there, too. Here are the rules being applied to the body:

@media (min-width: 768px) {
    body {
        transform: scale(0.75);
        -webkit-transform: scale(0.75);
        -moz-transform: scale(0.75);
        -ms-transform: scale(0.75);
        transform-origin: center 0;
        -webkit-transform-origin: center 0;
        -moz-transform-origin: center 0;
        -ms-transform-origin: center 0;

    }
}

In my case, the element is above the button that should be triggering zeroclipboard. I can click on the button, find the Flash object using the browser's inspector, and then click on that to trigger the copy action.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants