Opened 18 years ago
Closed 18 years ago
#117 closed Bug (fixed)
FCK.IsDirty() fails in Source view
Reported by: | Alfonso Martínez de Lizarrondo | Owned by: | Alfonso Martínez de Lizarrondo |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | General | Version: | FCKeditor 2.4 |
Keywords: | HasPatch | Cc: |
Description
The check is run against EditorDocument so now that the object is destroyed it raises an error, but previously it did return a wrong state while the user changed the source in the textarea.
IsDirty : function() { return ( this.StartupValue != this.EditorDocument.body.innerHTML ) ; }, ResetIsDirty : function() { if ( this.EditorDocument.body ) this.StartupValue = this.EditorDocument.body.innerHTML ; },
Attachments (1)
Change History (7)
comment:1 follow-up: 3 Changed 18 years ago by
comment:2 Changed 18 years ago by
Sorry, the last pseudocode is obviously wrong, I copy-pasted without changing the !=
StartupValue : '', HtmlStartupValue : '', IsDirty : function() { if ( FCK.EditMode == FCK_EDITMODE_SOURCE ) return ( this.HtmlStartupValue != FCK.EditingArea.Textarea.value ) ; else return ( this.StartupValue != this.EditorDocument.body.innerHTML ) ; }, ResetIsDirty : function() { if ( FCK.EditMode == FCK_EDITMODE_SOURCE ) { if ( this.EditorDocument.body ) { this.StartupValue = this.EditorDocument.body.innerHTML ; this.HtmlStartupValue = this.GetXHTML() ; } } else { this.StartupValue = What can we put here ??? this.HtmlStartupValue = FCK.EditingArea.Textarea.value ; } },
comment:3 Changed 18 years ago by
Keywords: | HasPatch added |
---|---|
Version: | → FCKeditor 2.4 |
Replying to alfonsoml:
The check gets the innerHTML of the body (that can be different to the provided source and to the HTML that it's output, just due to browser parsing and the HTML beautification)
So if we try to compare this.StartupValue (set while in WYSIWYG) and the textarea value it will fail most of the times although the user hasn't changed anything.
You are right. This is why ResetIsDirty is called right after the HTML loading on startup.
The problem is that we do that only on startup, not when switching mode.
Checking the value of FCK.GetXHTML might give a better check, but it will be more expensive and FCKXHtml.GetXHTML does a call itself to FCK.IsDirty, so it has to be protected to avoid an infinite loop.
You are right. The solution is not here.
It's not possible to update this.StartupValue when the user switch to source mode because then the IsDirty check wouldn't be run against the original HTML, but against the HTML that existed in the moment the user switched modes.
You are right, and here we have the base concept of the IsDirty feature to help us.
As you yourself have noted, if we just get the input, and compare it with the innerHTML, it will always be dirty. But developers looking to use the IsDirty want to be "notified" by changes made by "the user" to the document. They don't really expect the editor to make changes to it by itself. So, IsDirty is there to signalize a user action in the editor, not really a difference from the input and the output.
So, if a user opens a document, switch to source ten times and check IsDirty, the editor must say that anything has been done. If the user then changes a single space in the text (or in the source), then it will be dirty.
This is why it would be ok to ResetIsDirty when switching mode, much like we do in FCKXHtml.GetXHTML, making the fix quite simple.
Well... not that simple. The problem is that the EditorDocument is not available right after calls to SetHTML. So, a asynchronous solution must be found.
I'm attaching a proposal patch for it. It also adds a "resetIsDirty" param to SetHTML, making the previous start only code more generic, and making the asynchronous magic work.
Let me know your thoughts about it, Alfonso.
Changed 18 years ago by
Attachment: | isdirty.patch added |
---|
comment:4 Changed 18 years ago by
The only problem that I can think of that solution is that if the user does some change, then switch modes and undo it, the IsDirty will remain true all the way, but I didn't found any better solution.
So I think that until someone comes out with something better we must include it as it is completly broken as soon as the user switches to Source mode.
comment:5 Changed 18 years ago by
Ok, the patch solves the ticket problem. I think we should proceed with it.
I believe the undo combination could happen on very few cases, and actually an IsDirty inaccuracy is not a big issue on most sites that use it. In this way we at least reduce the problem occurrence to very few (unfortunate) cases.
Then we can open a dedicated ticket to the undo issue, which I already have a few ideas about (saving the IsDirty state with the Undo snapshot). I'm just worried about the performance of it.
comment:6 Changed 18 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
I'm trying to think how to make it work properly but it doesn't seem easy.
The check gets the innerHTML of the body (that can be different to the provided source and to the HTML that it's output, just due to browser parsing and the HTML beautification)
So if we try to compare this.StartupValue (set while in WYSIWYG) and the textarea value it will fail most of the times although the user hasn't changed anything.
Checking the value of FCK.GetXHTML might give a better check, but it will be more expensive and FCKXHtml.GetXHTML does a call itself to FCK.IsDirty, so it has to be protected to avoid an infinite loop.
It's not possible to update this.StartupValue when the user switch to source mode because then the IsDirty check wouldn't be run against the original HTML, but against the HTML that existed in the moment the user switched modes.
Maybe what we can do is to add another reference value: save the cleaned HTML source and depending on the editing mode do the check against one or another.
Pseudocode to explain this idea:
This can take care of returning the right IsDirty() value in both modes, but only if ResetIsDirty is called while in WYSIWYG. If it's called while in source we can get the HtmlStartupValue from the textarea value, but how do we get the StartupValue?