Opened 6 years ago

Closed 6 years ago

#9872 closed Bug (fixed)

checkDirty() function returns true when called on onload.

Reported by: parag patel Owned by: Piotrek Koszuliński
Priority: Normal Milestone: CKEditor 4.0.2
Component: General Version: 4.0 Beta
Keywords: Cc: bhavesh.khatri@…, parag.patel@…

Description

I have seen that when we call function checkDirty() on page load/ckeditor load for ckeditor version 4.0 it provides true value. This should be false. As at the first time of loading there could not be any change on content of loaded ckeditor, consequently it should return false. It is found that in older version ie v 3.6.3/3.6.5 above function call on page load gives undefined value. Please advise.

Thanks, Bhavesh khatri

Attachments (2)

Copy of api.html (4.4 KB) - added by parag patel 6 years ago.
api2.html (1.8 KB) - added by Jakub Ś 6 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 6 years ago by Jakub Ś

Keywords: checkDirty removed
Status: newpending

Could you please provide reduced and working sample that shows this problem?

I'm talking about HTML page that can be put in samples folder and will show the problem.

comment:2 in reply to:  1 Changed 6 years ago by parag patel

Replying to j.swiderski:

Could you please provide reduced and working sample that shows this problem?

I'm talking about HTML page that can be put in samples folder and will show the problem.

Attached file is edited version of api.html folder. You can put this file directly in samples folder and run it. On onload function checkdirty is called and it is returning true, which should be undefined as it was the case in previous version.

Changed 6 years ago by parag patel

Attachment: Copy of api.html added

comment:3 Changed 6 years ago by Jakub Ś

Status: pendingconfirmed
Version: 4.04.0 Beta

The problem can be reproduced from CKEditor 4 Beta.

To reproduce put api2.html in samples folder and load the page in browser.

CKEditor 3.6.5
Onload undefined
PluginsLoaded undefined
After PluginsLoaded undefined
InstanceReady false
After InstanceReady false

CKEditor 4 Beta
Onload true
PluginsLoaded true
After PluginsLoaded true
InstanceReady true
After InstanceReady true

Changed 6 years ago by Jakub Ś

Attachment: api2.html added

comment:4 Changed 6 years ago by Piotrek Koszuliński

Commit that changed this behaviour: git:8d0976d.

New behaviour seems a little bit weird, but I think that rationale behind it could be that when editor loads data it can fix them and since it's impossible to compare fixed data with the data passed as initial content (because in 90% cases they will be differently formatted) it's safer to set dirty == true. But to be sure let's wait for Fred's answer.

comment:5 Changed 6 years ago by parag patel

i am not using ckeditor 4 beta . i am using ckeditor_4.0_full . so please consider it.

comment:6 Changed 6 years ago by Jakub Ś

Version indicates when problem started occurring (Please remember that all users out there). Fix will be of course applied to the newest master version on git.

comment:7 Changed 6 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.0.2
Owner: set to Piotrek Koszuliński
Status: confirmedassigned

I figured out that this is unintended change. Test that checks dirty flag is incomplete.

comment:8 Changed 6 years ago by Piotrek Koszuliński

Status: assignedreview

I pushed two solutions on t/9872 and t/9872b plus tests to t/9872.

As tests prove during initialization editor was dirty. That was because resetDirty() has been called after firing instanceReady.

  • t/9872 is brutal but simple solution - it resets dirty flag twice during initialization, because data are changed twice.
  • t/9872b is more subtle - it forces dirty=false during initialization.
  • t/9872c - I haven't pushed this branch, but I've got a third idea. We may introduce editor#state (string - but I don't have idea what states we have except 'ready' :D) or editor#ready (boolean) flag which is IMO missing. Then fix'd be similar to t/9872b.

comment:9 Changed 6 years ago by Frederico Caldeira Knabben

Status: reviewreview_passed

KISS: R+ for t/9872.

comment:10 Changed 6 years ago by Olek Nowodziński

Resolution: fixed
Status: review_passedclosed

Fixed with git:8f62b004fe76 and b1a3f3f0fd9c189@tests-v4.

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy