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.
Description
Details
- Reference
- bz18654
Event Timeline
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.
Change 596851 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] [EditPage] Add new EditViewer context, start using in EditPage
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
Change 597412 merged by jenkins-bot:
[mediawiki/core@master] [EditPage] Move AS_* status constants to a new interface
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
Change 609628 merged by jenkins-bot:
[mediawiki/core@master] [EditPage] Mark public fields not used outside of core as internal
You may want to consider the editing APIs as part of this. The editing team had to create ApiVisualEditor(Edit).php to fetch all the data needed for a full editing experience (e.g. loading edit checkboxes and other metadata), and in places we had to expose methods in EditPage to achieve this.
Yeah, I'm keeping that in mind as a work - T252962 is for ApiVisualEditor using EditPage for just one thing
Removing task assignee due to inactivity as this open task has been assigned for more than two years. See the email sent to the task assignee on August 22nd, 2022.
Please assign this task to yourself again if you still realistically [plan to] work on this task - it would be welcome!
If this task has been resolved in the meantime, or should not be worked on ("declined"), please update its task status via "Add Action… 🡒 Change Status".
Also see https://www.mediawiki.org/wiki/Bug_management/Assignee_cleanup for tips how to best manage your individual work in Phabricator. Thanks!