Ticket #711 (closed enhancement: wontfix)

Opened 20 months ago

Last modified 4 months ago

separate function for onunload event

Reported by: FND Owned by: MartinBudden
Priority: minor Milestone: 2.6
Component: core Version:
Severity: trivial Keywords:
Cc:

Description

Currently, TiddlyWiki embeds the code for the onunload event directly in the HTML element: 

<body onload="main();" onunload="if(window.checkUnsavedChanges) checkUnsavedChanges(); if(window.scrubNodes) scrubNodes(document.body);">

This should be moved to a separate function so it can be hijacked/overridden.

Change History

  Changed 12 months ago by MartinBudden

  • owner changed from JeremyRuston to MartinBudden
  • status changed from new to assigned

follow-up: ↓ 3   Changed 11 months ago by MartinBudden

  • status changed from assigned to closed
  • resolution set to fixed

Fixed in changeset:9473

in reply to: ↑ 2   Changed 11 months ago by EricShulman

Changing the function call from checkUnsavedChanges() to unload() breaks any existing plugins that currently hijack checkUnsavecChanges() to add extra behaviors to the 'onunload' handling.

Although this change *is* both reasonable and appropriate, the impact on existing usage should be clearly noted, so that plugin code can be updated pro-actively, rather than by discovering broken functionality after the new core code is released.

follow-up: ↓ 5   Changed 11 months ago by MartinBudden

Eric,

I don't think this will break any existing code, since unload() calls checkUnsavedChanges(), so any code that hijacks checkUnsavedChanges() will still be called.

Can you clarify?

Martin

in reply to: ↑ 4   Changed 11 months ago by EricShulman

Replying to MartinBudden:

I don't think this will break any existing code, since unload() calls checkUnsavedChanges(), so any code that hijacks checkUnsavedChanges() will still be called.

umm... yeah. You're right. This particular change should be 'safe' for existing hijacks. I guess I'm just being very cautious and hyper-vigilant regarding possible effects of core changes, especially when refactoring functions and adding new API.

  Changed 8 months ago by JeremyRuston

  • status changed from closed to reopened
  • resolution fixed deleted

Looking at the unload() function, I don't think the defensive checks are necessary any longer, it could just be: 

function unload()
{
        checkUnsavedChanges();
         scrubNodes(document.body);

  Changed 8 months ago by FND

  • milestone changed from 2.5.1 to 2.5.3

  Changed 8 months ago by FND

  • milestone changed from 2.5.3 to 2.5.4

  Changed 4 months ago by FND

  • status changed from reopened to closed
  • resolution set to wontfix

wontfix; there might be browser quirks which depend on the current implementation

Note: See TracTickets for help on using tickets.