Differential D12825

Fix recalculating forms twice
ClosedPublic
Actions

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

Details

Reviewers
aacid
Commits
R223:3ae2ae06e810: Fix recalculating forms twice
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
aheinecke added a comment.May 11 2018, 12:50 PM

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

aheinecke added a comment.May 17 2018, 3:07 PM

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.

aacid added a comment.May 18 2018, 11:14 AM

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

aheinecke added a comment.May 18 2018, 11:17 AM

Ugh, Sorry for overlooking these.

Yes. They also call notifyFormChanges in their redo.

aacid added a comment.May 18 2018, 12:40 PM

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

aheinecke added a comment.May 23 2018, 11:56 AM

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
Closed by commit R223:3ae2ae06e810: Fix recalculating forms twice (authored by aheinecke, committed by aacid). · Explain WhyMay 28 2018, 2:30 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mcore/document.cpp (6 lines)

Diff 35029

View Options

core/document.cpp

Loading...