mootools / mootools-core

MooTools Core Repository
https://mootools.net
2.65k stars 507 forks source link

Element.prototype.getPosition does not properly handle elements with fixed position parents #2766

Open Continuities opened 8 years ago

Continuities commented 8 years ago

It looks like the intent is for getPosition to return a number relative to the viewport for fixed position elements. If an element is not fixed but nested inside a fixed element, however, getPosition returns a number relative to the document. This is because isFixed (on line 136 of Element.Dimensions.js) only checks the styling of the element itself.

The problem is worse because the behaviour is inconsistent between desktop and iOS. iOS uses a different code-path in getOffsets that correctly returns a number relative to the viewport for elements inside fixed elements.

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/32112238-element-prototype-getposition-does-not-properly-handle-elements-with-fixed-position-parents?utm_campaign=plugin&utm_content=tracker%2F22067&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F22067&utm_medium=issues&utm_source=github).
SergioCrisostomo commented 8 years ago

@Continuities thanks for reporting this.

If you could provide a jsFiddle or example that reproduces the problem would be great.

Cheers

Continuities commented 8 years ago

https://jsfiddle.net/Continuities/bo1zm146/3/

  1. Run fiddle in Chrome on desktop and scroll the view. Note that the number (getPosition().y of the target element) goes up, representing a position relative to the document.
  2. Spoof an iOS useragent (don't actually load jsFiddle on an iPhone. The layout doesn't work properly), and reload. Scroll the view. Note that the number stays 0, representing a position relative to the viewport.
ciez commented 7 years ago

afaik this is expected behaviour. getPosition is calculated re: relative positioned parent.

CSS behaves smilarly: offset is specified re: relative positioned parent.

Continuities commented 7 years ago

It's expected that getPosition should behave differently between iOS and desktop Chrome?

ciez commented 7 years ago

top div is rarely set position:fixed. This may be allowed however this may be causing the issue.

I just tried to wrap entire html in another generic <div> (position:inherited/not set), spoofed IPad and IPhone 6.

behaviour is similar to desk Chrome: the number is updated.

just remember that position get / set works relative to nearest non-fixed parent. This is aligned with CSS.

Continuities commented 7 years ago

Not the behaviour I'm seeing. Wrapping the entire thing in an unstyled div does nothing to change the inconsistency between desktop and iOS. I know exactly where the bug is, and reported it in the original issue description.

ciez commented 7 years ago

how do you spoof iOS?

what if you getOffsetParent and pass the parent to getPosition? does it work as you need it?

if position is calculated relative to any parent regardless of their fixed / relative attribute, this would lead to different behaviour from CSS and from how it works now.

Continuities commented 7 years ago

Chrome dev tools with an iOS user-agent. I don't really care which behaviour you choose (either document-, or viewport-relative), I just want it to be consistent between platforms. It should not be necessary to implement workarounds (like getOffsetParent) to achieve consistency.

ciez commented 7 years ago

I use toggle device toolbar. Is this the same thing?

actually, even without adding generic div, behaviour is similar.

there may be differences in Chrome versions. Do you use Linux too?

also (just checking) did you disable cache in dev-tools?

Continuities commented 7 years ago

Are you reloading between switching devices? The user-agent is only actually updated if you refresh. It's not a Chrome issue, it's caused by the two different code-paths in getOffsets behaving differently:

The first block (inside the if (hasGetBoundingClientRect) conditional) only checks the current element for position:fixed. This is the block that desktop uses. The second block walks up the DOM and adds offsets all the way up. This is the block that iOS uses.

I can fix the issue myself, if necessary. I just don't want to bother cloning the project and making a pull request.

ciez commented 7 years ago

I tried to reload.

js fiddle code (not your snippet, jsfiddle editor js file) throws an error. Scrolling behaviour after this error is very odd: the scroll hand moves in the opposite direction from cursor. I suppose it may be caused by jsfiddle's error.

As a side note: I am not Mootools library dev - a user like yourself. I remembered I was surprised to find out how position fixed and relative are different. Thought I could help.

To check your code, I need to load your example outside of jsFiddle. Let me do this in the next few days.

If you change the library, please do not change desktop Chrome behaviour.

Continuities commented 7 years ago

the scroll hand moves in the opposite direction from cursor

That just sounds like it's switched to touch-based scrolling. There is a jsFiddle error thrown, but it doesn't appear to cause any problems.

If you change the library, please do not change desktop Chrome behaviour.

See, that's the issue. I think iOS is behaving correctly in this instance, and not desktop Chrome. Behaviour could be consolidated to be "wrong" everywhere, but that doesn't feel like the right answer. I'll leave it up to the MooTools dev community.

ciez commented 7 years ago

just noticed that https://jsfiddle.net/Continuities/bo1zm146/3/ does not use MooTools. Javascript is set to No-library (pure Js)

is this intended? Are we testing MooTools then?

Continuities commented 7 years ago

It's an external dependency. None of the code would work without MooTools.

Continuities commented 7 years ago

While trying to write a spec to capture this behaviour, I discovered another interesting quirk: The bug disappears if the scrolling element is any element other than the window (ie, the fixed malarky is inside a div with overflow:scroll instead of directly inside the window). Looks like element scrolls are subtracted out, while html scrolls are not.

ciez commented 7 years ago

do not iOS capture other event, not scroll?

I modified your code to this

iOS (drag?) behaves similar to desktop scroll upd: I had forgot to reload. iOS shows 0.

Continuities commented 7 years ago

That code still exhibits the inconsistent behaviour that I described. I don't think you're properly emulating iOS user-agents.

ciez commented 7 years ago

I overlooked that sticky has position fixed and thought about absolute vs relative / not set attributes.

I am not sure what getPosition should return for fixed elements and their children. I'd never tried it.

Sorry about the confusion. I can't help.

Continuities commented 7 years ago

It's alright =)