#9491 open bug ()

Opened August 09, 2013 10:42AM UTC

Last modified July 14, 2014 06:43PM UTC

DatePicker does not use the correct ownerDocument (does not support multiple windows)

Reported by: lukepage Owned by:
Priority: minor Milestone: none
Component: ui.datepicker Version: 1.10.3
Keywords: rewrite Cc:
Blocked by: Blocking:
Description

This issue came up here

http://bugs.jqueryui.com/ticket/4908

It was however closed as saying most issues are fixed. However the datepicker control frequently decides to request a specific id (e.g. $(id)) without specifying a document. This makes the datepicker unusable on a 2nd window.

I need this fixed and will happily provide a patch if I can get some feedback on the correct fix for this.

Attachments (0)
Change History (7)

Changed August 09, 2013 10:57AM UTC by lukepage comment:1

so..

here https://github.com/jquery/jquery-ui/blob/master/ui/jquery.ui.datepicker.js#L1575

why does it capture the id instead of inst? is it to stop memory leaks ( a reference from the function capture to the element).

One fix would be to pass the ownerDocument from e.target.ownerDocument in each of the event handles. what do people think about that?

Changed August 09, 2013 11:50AM UTC by scottgonzalez comment:2

status: new → open

I only looked at a few of the methods where id is being passed around, but it looks like they all just do $(id) right away. So it should be safe to just pass the DOMElement instead of the id. You'll need to check if id is being used anywhere else in those functions though.

When #6228 is fixed this problem will just naturally go away. But if you send a PR fixing all of the queries now, we'd accept that.

Changed August 09, 2013 02:27PM UTC by lukepage comment:3

patch added here

https://github.com/jquery/jquery-ui/pull/1052

Changed August 09, 2013 07:18PM UTC by tj.vantoll comment:4

Replying to [comment:3 lukepage]:

patch added here https://github.com/jquery/jquery-ui/pull/1052

@lukepage Could you please create a test case that shows this issue. I made one here http://jsfiddle.net/tj_vantoll/5za9s/ and it looks like your patch isn't handling for that.

Thanks.

Changed August 15, 2013 08:09AM UTC by lukepage comment:5

yes 2 issues here.. one was you were referencing the https version so it wasn't included.

The 2nd was that I tested it inside my application which wraps jquery ui a bit and appends it to the right thing.. jqueryUI appends the datepicker to the current window rather than the window with the control in it. I will have a look to see if I can patch that bit as well.

Changed August 15, 2013 08:21AM UTC by lukepage comment:6

It looks like a much bigger patch to fix the issue completely rather than patch the small area that is effecting my usage of the datepicker. Will have to think whether it is worth patching this or contributing a fix for #6228

Changed July 14, 2014 06:43PM UTC by scottgonzalez comment:7

keywords: → rewrite