zeroclipboard icon indicating copy to clipboard operation
zeroclipboard copied to clipboard

Position of Flash movie incorrect when CSS zoom is used

Open myitcv opened this issue 12 years ago • 37 comments

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

myitcv avatar May 10 '13 17:05 myitcv

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

JamesMGreene avatar May 10 '13 18:05 JamesMGreene

@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.

myitcv avatar May 10 '13 18:05 myitcv

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

JamesMGreene avatar May 10 '13 18:05 JamesMGreene

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

myitcv avatar May 13 '13 22:05 myitcv

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

myitcv avatar May 13 '13 22:05 myitcv

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

JamesMGreene avatar May 14 '13 00:05 JamesMGreene

@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).

JamesMGreene avatar May 27 '13 03:05 JamesMGreene

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.

myitcv avatar May 27 '13 11:05 myitcv

Note to self: http://www.htmlgoodies.com/beyond/css/css3-features-every-web-designer-is-excited-about-skewed-rotated-and-scaled-text.html

JamesMGreene avatar Jun 03 '13 12:06 JamesMGreene

@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 avatar Jul 06 '13 17:07 JamesMGreene

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!

JamesMGreene avatar Jul 12 '13 12:07 JamesMGreene

@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?

myitcv avatar Jul 13 '13 16:07 myitcv

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 avatar Jul 13 '13 16:07 JamesMGreene

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.

myitcv avatar Aug 13 '13 15:08 myitcv

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

//spent 4 hours digging this =((

H1D avatar Aug 15 '13 18:08 H1D

@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 avatar Aug 15 '13 18:08 JamesMGreene

@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 avatar Aug 15 '13 20:08 JamesMGreene

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. :wink:

JamesMGreene avatar Aug 15 '13 20:08 JamesMGreene

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

JamesMGreene avatar Aug 15 '13 20:08 JamesMGreene

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

H1D avatar Aug 15 '13 21:08 H1D

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?

JamesMGreene avatar Aug 16 '13 15:08 JamesMGreene

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

myitcv avatar Aug 16 '13 15:08 myitcv

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

JamesMGreene avatar Aug 16 '13 15:08 JamesMGreene

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

JamesMGreene avatar Aug 16 '13 16:08 JamesMGreene

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.

myitcv avatar Aug 16 '13 16:08 myitcv

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.

jonrohan avatar Aug 16 '13 16:08 jonrohan

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 avatar Aug 16 '13 16:08 JamesMGreene

CSS zoom is a damn mess cross-browser: http://jsfiddle.net/JamesMGreene/2Rn3F/

https://twitter.com/_JamesMGreene/status/368474804606623745

JamesMGreene avatar Aug 16 '13 20:08 JamesMGreene

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. :confused:

JamesMGreene avatar Oct 17 '13 14:10 JamesMGreene

@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

myitcv avatar Nov 10 '13 10:11 myitcv