Opened 17 years ago

Closed 11 years ago

#4045 closed Bug (fixed)

some admin Javascript fixes and cleanups

Reported by: arvin Owned by: andrewjesaitis
Component: contrib.admin Version: dev
Severity: Normal Keywords: admin calendar
Cc: andrewjesaitis Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

I recognised that the calendar behaves a bit different in Konqueror than in
Firefox: When clicking inside the calendar but not on a element with
functionality (e.g. month name) the calendar is closed in Konqueror but not in
Firefox. This lead me to the first part of my patch:

  • Attach "onclick" events to "document" instead of "window". Konqueror does not deliver the event to "window" even when the propagation was stopped. Internet Explorer (tested with IE6 SP2 at a friend computer) does not deliver the event at all to "window". AFAIK "window" in not part of DOM so "document" should be more portable. I have tested the new code with Firefox, Explorer, Safari, Konqueror and Opera.

The other parts are only minor cleanups:

  • Event handlers set in openClock/openCalendar are removed in dismissClock/dismissCalendar ("window.onclick = null;" does not work since handlers are installed with addEvent).
  • Calendar inserts dates with leading zeroes.
  • Remove duplicates of functions "quickElement" and "addEvent".
  • Move "removeChildren" and "cancelEventPropagation" to "core.js" as they might be handy also at other places.

If you want a patch with only some parts included let me know.

Attachments (1)

admin-js.diff (6.9 KB ) - added by arvin 17 years ago.

Download all attachments as: .zip

Change History (13)

by arvin, 17 years ago

Attachment: admin-js.diff added

comment:1 by Simon G. <dev@…>, 17 years ago

Component: UncategorizedAdmin interface
Owner: changed from Jacob to Adrian Holovaty
Triage Stage: UnreviewedReady for checkin

comment:2 by Malcolm Tredinnick, 17 years ago

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

I believe we still support IE5.5/Win as much as possible, so I'd like to hear that this patch works there, too, before checking in.

Sprry to be a pain, but please split the patch into two parts: changes that don't affect any functionality (the "clean-ups") and changes that fix an actual bug. Make a new ticket for the clean-up portions. That way we can evaluate the real problem-fixes in isolation.

comment:3 by arvin, 17 years ago

The patch with the pure cleanup is in ticket #4120.

Since a patch with the pure changes would conflict I thinks it's best I make a new
patch against the SVN for this ticket when the other thing is through.

comment:4 by James Bennett, 17 years ago

Owner: changed from nobody to xian

Reassigning to xian since he's doing admin JS stuff.

comment:5 by Adam Nelson, 14 years ago

Owner: changed from xian to nobody

xian obviously isn't doing anything with this. Patch needs to be updated to current js.

comment:6 by Łukasz Rekucki, 13 years ago

Severity: Normal
Type: Bug

comment:7 by anonymous, 13 years ago

Easy pickings: unset
UI/UX: unset

A js bug-fix for a 4 year old browser version? Perhaps this should just be silently closed?

comment:8 by Aymeric Augustin, 11 years ago

Much of the patch still applies (with cd django/contrib/admin/static/admin/js; patch -p0 < admin-js.diff).

comment:9 by andrewjesaitis, 11 years ago

Cc: andrewjesaitis added
Owner: changed from nobody to andrewjesaitis
Status: newassigned

Picking this up, but ticket #4120 is a prerequisite, so I'll wait on doing anything until that ticket is resolved. However, it looks like most of the things that need to be fixed -- attaching events to the document instead of window, have occurred during the intervening years. The only remaining thing would be to "more properly" remove the events using core.js removeEvent function rather that just setting window.document.onclick = null.

comment:10 by andrewjesaitis, 11 years ago

This is the remaining piece of the patch for ticket 4045. All we are doing is using the core.js removeEvent to unbind the dismissClock and dismissCalendar functions from the click event. https://github.com/django/django/pull/989

comment:11 by Claude Paroz <claude@…>, 11 years ago

In 4509a1be1f1164164c2b654d1e3e4169ef71b5ae:

Explicitly removes dismissClock

Uses the removeEvent function in core.js to remove the function from
the document click event.
Refs #4045.

comment:12 by Claude Paroz <claude@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In 5ab66dea50eab6625e3a07b71342268432e9cfaf:

Explicitly removes dismissCalendar

Uses the removeEvent function in core.js to remove the dismissCalendar
function from the document click event.
Fixes #4045.

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