Ticket #711 (closed enhancement: wontfix)

Opened 4 years ago

Last modified 2 years 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 3 years ago by MartinBudden

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

follow-up: ↓ 3   Changed 3 years ago by MartinBudden

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

Fixed in changeset:9473

in reply to: ↑ 2   Changed 3 years 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 3 years 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 3 years 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 3 years 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 3 years ago by FND

  • milestone changed from 2.5.1 to 2.5.3

  Changed 3 years ago by FND

  • milestone changed from 2.5.3 to 2.5.4

  Changed 2 years 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.