Page MenuHomePhabricator

EditPage.php needs rewrite: Separate DB and UI logic
Open, MediumPublic

Description

The code in EditPage.php doesn't separate DB and UI logic properly, and desperately needs to be totally rewritten. This lack of separation means, among other things, that calling the API action=edit module internally from a SpecialPage is horribly broken and that the API action=edit code itself is ugly.

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 10:36 PM
bzimport set Reference to bz18654.
bzimport added a subscriber: Unknown Object (MLST).
Catrope created this task.May 2 2009, 2:34 PM

I'd like to add that customising the edit page, or rather using its components elsewhere is very hard if not impossible.

happy.melon.wiki wrote:

I might try rewriting it as a special page? [[Special:EditPage]]?? Cf bug18789, bug11456, etc. Would that have a reasonable likelihood of being implemented if it worked? With the write API now well-developed, anything that breaks from making &action=edit redirect to Special:EditPage *deserves* to break. We'd need to fix bug18789 for UI consistency, but that shouldn't be too difficult (should probably be implemented in SpecialPage so it can be used by SpecialMovePage, SpecialStabilization, etc etc).

(In reply to comment #2)

I might try rewriting it as a special page? [[Special:EditPage]]?? Cf
bug18789, bug11456, etc. Would that have a reasonable likelihood of being
implemented if it worked? With the write API now well-developed, anything that
breaks from making &action=edit redirect to Special:EditPage *deserves* to
break.

Sounds like a plan, as long as such an implementation separates UI and DB logic properly, with the former going into SpecialEditPage.php and the latter in something like Edit.php (the point being that the API should be able to do all its edit stuff through the Edit class without needing the SpecialEditPage class).

Would it also be possible to finally separate section title input from edit summary input? Right now they are both set with the same input, which makes the API syntax confusing.

I don't think reducing technical debt counts as an enhancement request.

Futzing with the bug settings accordingly.

Krinkle set Security to None.
Krinkle removed a subscriber: Unknown Object (MLST).
awight added a subscriber: awight.Mar 14 2015, 10:10 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 23 2016, 12:08 AM
Ricordisamoa added a subscriber: Ricordisamoa.
Retro added a subscriber: Retro.Jul 2 2019, 5:34 PM
Restricted Application added a project: User-DannyS712. · View Herald TranscriptApr 22 2020, 12:38 AM
Aklapper renamed this task from EditPage.php needs rewrite to EditPage.php needs rewrite: Separate DB and UI logic.Apr 22 2020, 8:47 AM

Change 596851 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] [EditPage] Add new EditViewer context, start using in EditPage

https://gerrit.wikimedia.org/r/596851

DannyS712 updated the task description. (Show Details)May 17 2020, 3:53 AM

Unused hooks that should be removed as part of the cleanup:

  • EditPageGetDiffContent
  • EditPageTosSummary (1 registered handler in a github repo extension that is an empty function)[1]

[1] Needed to move EditPage::showTosSummary to EditViewer without needing to inject a HookRunner

Methods that are candidates to move to EditViewer without requiring EditViewer to be a service with dependencies injected, and not covered in the initial patch (only used in action=edit, not api):

  • displayPermissionsError
    • Will need current content (EditPage::getContentObject) passed
  • showTextbox
  • addEditNotices
    • Will need oldid passed
  • addTalkPageText
  • addLongPageWarningHeader
    • Will need content length passed

Change 597412 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] [EditPage] Move AS_* status constants to a new interface

https://gerrit.wikimedia.org/r/597412

Change 597412 merged by jenkins-bot:
[mediawiki/core@master] [EditPage] Move AS_* status constants to a new interface

https://gerrit.wikimedia.org/r/597412

Change 609628 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] [EditPage] Mark public fields not used outside of core as internal

https://gerrit.wikimedia.org/r/609628

Change 609628 merged by jenkins-bot:
[mediawiki/core@master] [EditPage] Mark public fields not used outside of core as internal

https://gerrit.wikimedia.org/r/609628