Fix recalculating forms twice
ClosedPublic

Authored by aheinecke on May 11 2018, 12:46 PM.

Details

Summary

As a side effect of f0a80a67 a recalculation was triggered
by the notifyFormChanges called in EditFormTextCommand::redo,
which is called for each edit. So calculation was done twice.

Test Plan

The calculate forms test still passes.

Diff Detail

Repository
R223 Okular
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
aheinecke created this revision.May 11 2018, 12:46 PM
Restricted Application added a project: Okular. · View Herald TranscriptMay 11 2018, 12:46 PM
Restricted Application added a subscriber: okular-devel. · View Herald Transcript
aheinecke requested review of this revision.May 11 2018, 12:46 PM

I did not know when adding f0a80a67 that pushing to the undoStack automatically executes the redo function.

To clarify:

d->m_undoStack->push( uc );

results in:
EditFormTextCommand::redo()

https://cgit.kde.org/okular.git/tree/core/documentcommands.cpp#n513

which calls:

m_docPriv->notifyFormChanges( m_pageNumber );

https://cgit.kde.org/okular.git/tree/core/document.cpp#n3435

That triggers the recalculate. So it is duplicated here.

Should we remove the recalculateForms from editFormList and editFormCombo too then?

Ugh, Sorry for overlooking these.

Yes. They also call notifyFormChanges in their redo.

Will you upload an updated patch or should i just do it ?

I'll upload a new patch. (sorry for delaying my okular work, I'm currently still a bit swamped by the efail hype)

aheinecke updated this revision to Diff 35008.May 28 2018, 7:34 AM

Include duplicate cals from editFormList and editFormCombo

aacid accepted this revision.May 28 2018, 2:04 PM
This revision is now accepted and ready to land.May 28 2018, 2:04 PM
This revision was automatically updated to reflect the committed changes.