Opened 9 years ago

Closed 3 years ago

#4045 closed Bug (fixed)

some admin Javascript fixes and cleanups

Reported by: arvin Owned by: andrewjesaitis
Component: contrib.admin Version: master
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


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 9 years ago.

Download all attachments as: .zip

Change History (13)

Changed 9 years ago by arvin

comment:1 Changed 9 years ago by Simon G. <dev@…>

  • Component changed from Uncategorized to Admin interface
  • Needs documentation unset
  • Needs tests unset
  • Owner changed from jacob to adrian
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Ready for checkin

comment:2 Changed 8 years ago by mtredinnick

  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to Accepted

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 Changed 8 years ago by arvin

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 Changed 8 years ago by ubernostrum

  • Owner changed from nobody to xian

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

comment:5 Changed 5 years ago by adamnelson

  • Owner changed from xian to nobody

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

comment:6 Changed 5 years ago by lrekucki

  • Severity set to Normal
  • Type set to Bug

comment:7 Changed 4 years ago by anonymous

  • 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 Changed 3 years ago by aaugustin

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

comment:9 Changed 3 years ago by andrewjesaitis

  • Cc andrewjesaitis added
  • Owner changed from nobody to andrewjesaitis
  • Status changed from new to assigned

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 Changed 3 years ago by andrewjesaitis

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.

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

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 Changed 3 years ago by Claude Paroz <claude@…>

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

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