Opened 8 years ago

Closed 8 years ago

#5067 closed (fixed)

Calendar and clock popups are wrongly positioned if within a scrollable div

Reported by: erichs@… Owned by: rcoup
Component: contrib.admin Version: master
Severity: Keywords: DateTimeShortcuts calendar clock admin scrolling position
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

The calendar and clock popups created through DateTimeShortcuts.js are positioned relative to cumulative offset (left and top) of all parent objects. This doesn't work if the firing up the popup is within a scrollable div. The popups get misaligned by the actual distances scrolled. This is very easily fixed by subtracting the scrolled distance from the objects' offsets whilst iterating through.

See attached patch of core.js.

Attachments (6)

patch.diff (889 bytes) - added by erichs@… 8 years ago.
patch of core.js to cater for scrolling when calculating positions
patch.2.diff (987 bytes) - added by Erich Schmid <erichs@…> 8 years ago.
updated path with fix for IE7
patch.3.diff (1.4 KB) - added by Erich Schmid <erichs@…> 8 years ago.
Updated Patch, has now been tested on IE6/7, Firefox 2.0.0.6 and Safari 3.0.3 using the default admin view and a scrollable fixed positioned div (both scrolled and not scrolled)
locationtest.html (4.1 KB) - added by russellm 8 years ago.
Demonstration of the problem, and the fix in action.
5067-patch.4.diff (1.6 KB) - added by rcoup 8 years ago.
Nicer browser identification & Opera support
locationtest.2.html (4.2 KB) - added by rcoup 8 years ago.
Updated testcase

Download all attachments as: .zip

Change History (13)

Changed 8 years ago by erichs@…

patch of core.js to cater for scrolling when calculating positions

comment:1 Changed 8 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from adrian to russellm
  • Patch needs improvement unset

Changed 8 years ago by Erich Schmid <erichs@…>

updated path with fix for IE7

comment:2 Changed 8 years ago by erichs@…

Please hold on for a while. The current patch does not work on simpler pages like the admin pages. Still working on a patch that works on all browsers - if that's actually possible.

Changed 8 years ago by Erich Schmid <erichs@…>

Updated Patch, has now been tested on IE6/7, Firefox 2.0.0.6 and Safari 3.0.3 using the default admin view and a scrollable fixed positioned div (both scrolled and not scrolled)

Changed 8 years ago by russellm

Demonstration of the problem, and the fix in action.

comment:3 Changed 8 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

The attached locationtest.html demonstrates the problem. It contains a copy of the existing findXPos and findYPos, plus the fixed versions from the patch. However, I'm not too happy with the explicit user agent checking. Can anyone suggest a better approach here?

comment:4 Changed 8 years ago by rcoup

  • Owner changed from nobody to rcoup
  • Patch needs improvement set
  • Status changed from new to assigned

comment:5 Changed 8 years ago by Erich Schmid <erichs@…>

I'm having some difficulty with the reasoning here. It's an unfortunate fact that web browsers behave differently and hence it's not always possible to get everything working with one line of code. But leaving it broken for all browsers, doesn't seem like the best way to go. Practicality should rule over purity.

Changed 8 years ago by rcoup

Nicer browser identification & Opera support

Changed 8 years ago by rcoup

Updated testcase

comment:6 Changed 8 years ago by rcoup

The new patch (4) is better and still reasonably simple. Tested in Safari3, IE6, IE7, FF2, Opera9. Opera was broken in patch 3.

There are a number of edge cases where it won't work, but without going to a full-blown framework like Dojo you won't get there. I've put in a slightly safer & faster version in the testcase html, but its probably not necessary.

Still known broken:

  • R-to-L text direction in IE will put it backwards.
  • If the node is an absolutely positioned child of a body with a margin set in Safari

comment:7 Changed 8 years ago by russellm

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [6172]) Fixed #5067 -- Fixed a problem with javascript popup widgets appearing in the wrong place if they were in a overflow=scroll block. Thanks to Erich Schmid for the original fix, and Robert Coup for the updated patch.

Note: See TracTickets for help on using tickets.
Back to Top