Bug Tracker

Opened 9 years ago

Closed 9 years ago

#6968 closed bug (fixed)

"handle" won't work as a key for .data() method

Reported by: itachi Owned by: snover
Priority: blocker Milestone: 1.5
Component: data Version: 1.4.3
Keywords: needstest patch Cc:
Blocked by: Blocking: #7840

Description

--- to reproduce, evaluate:

/* any jQuery el */

var $el = $(document)

/* set "handle" to anything but a function */

$el.data("handle", {}))

/* this fails, see line 1850 where jQuery.trigger() function assumes that jQuery.data( elem, "handle" ) is a function */

$el.data("handle")

--- expected: "handle" keyword work or is marked invalid in docs

Change History (14)

comment:1 Changed 9 years ago by dmethvin

Good point, perhaps it would be a good idea for jQuery to use a leading $ for its data names to avoid conflicts.

comment:2 Changed 9 years ago by dmethvin

need: ReviewPatch

Also used internally:

__className__
handle
events
_change_data
lastToggle
events
olddisplay

comment:3 Changed 9 years ago by snover

Milestone: 1.4.31.next
Priority: blocker
Status: newopen
Version: 1.4.21.4.3

We need to stop polluting the data objects with internal data. 1.4.3 also adds __events__ as a polluting key.

comment:4 Changed 9 years ago by snover

Milestone: 1.4.41.5

Retargeting to 1.5.

comment:5 Changed 9 years ago by Rick Waldron

Keywords: needstests patch added

comment:6 Changed 9 years ago by jitter

Keywords: needstest added; needstests removed

comment:7 Changed 9 years ago by cobexer

what about using jQuery.expando + "_events",...

this would remove all collision possibilities.

however it would also pollute the display in FireQuery (Firebug addon)

maybe reserve "jQuery." as prefix a user may not use

comment:8 Changed 9 years ago by iliakan

It seems sane to have $ as prefix for all jquery native stuff, or even $, like: "" as hidden library stuff and "$" pointing that it is jquery library stuff.

comment:9 Changed 9 years ago by Rick Waldron

#7651 is a duplicate of this ticket.

comment:10 Changed 9 years ago by Rick Waldron

Blocking: 7840 added

comment:11 Changed 9 years ago by snover

Owner: set to snover
Status: openassigned

comment:12 in reply to:  8 Changed 9 years ago by gnarf

Replying to iliakan:

It seems sane to have $ as prefix for all jquery native stuff, or even $, like: "" as hidden library stuff and "$" pointing that it is jquery library stuff.

I kinda like that idea... .data('$')['events'] or .data('$events') either seems less polluting for the internals. I wonder how many people are using .data('events') or other internal keys for their own code/purposes though. Either suggestion would be easy enough to compat mode, making the .data strip the $ off the argument.

comment:13 Changed 9 years ago by snover

pull request i forgot to put this here.

comment:14 Changed 9 years ago by Colin Snover

Resolution: fixed
Status: assignedclosed

Merge in data_nocollide branch. Fixes #6968, improves unit testing framework checks for leaky stuff.

Changeset: e78d3a7e2d47e9796f87c18b76f8178b0bee42c5

Note: See TracTickets for help on using tickets.