Page MenuHomePhabricator

Reduce public variables and methods in EditPage
Closed, ResolvedPublic

Description

Currently, there is a large number of public methods and variables that shouldn't be used outside of the class (eg when the entire EditPage object is passed in a hook). These should be deprecated for public access and made private

Methods to make private*

  • EditPage::extractSectionTitle (used in EditPageTest)
  • EditPage::getSummaryInputWidget
  • EditPage::noSuchSectionPage
  • EditPage::initialiseForm
  • EditPage::setPreloadedContent (entirely unused)

*Only used in EditPage unless noted otherwise

Variables to make private

  • EditPage::$hookError
  • EditPage::$deletedSinceEdit
  • EditPage::$lastDelete
  • EditPage::$mTokenOkExceptSuffix
  • EditPage::$mTriedSave
  • EditPage::$incompleteForm
  • EditPage::$tooBig
  • EditPage::$missingComment
  • EditPage::$missingSummary
  • EditPage::$allowBlankSummary
  • EditPage::$autoSumm
  • EditPage::$hasPresetSummary
  • EditPage::$recreate
  • EditPage::$noSummary
  • EditPage::$parentRevId
  • EditPage::$editintro
  • EditPage::$markAsBot
  • EditPage::$mPreloadContent (public method setPreloadedContent exists, as does protected getPreloadedContent)

Event Timeline

Change 596791 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Make EditPage::$hookError private

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

Change 596791 merged by jenkins-bot:
[mediawiki/core@master] Make EditPage::$hookError private

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

Change 640490 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Reduce public methods of EditPage

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

Change 640490 merged by jenkins-bot:
[mediawiki/core@master] Reduce public methods of EditPage

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

Change 641504 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Add RELEASE-NOTES for removals of EditPage public methods

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

Change 641504 merged by jenkins-bot:
[mediawiki/core@master] Add RELEASE-NOTES for removals of EditPage public methods

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

I see several patches merged. Is this completed?

The variables need to be made private still

Change 642440 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Make @internal EditPage variables private

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

Change 647298 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Emit deprecation warnings for internal EditPage fields

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

Change 647298 merged by jenkins-bot:
[mediawiki/core@master] Emit deprecation warnings for internal EditPage fields

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

Change 761914 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] EditPage: make most methods private

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

Change 642440 merged by jenkins-bot:

[mediawiki/core@master] Remove public access to internal EditPage variables

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

Change 761914 merged by jenkins-bot:

[mediawiki/core@master] EditPage: make most methods private

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

Change 773856 had a related patch set uploaded (by Krinkle; author: Daimona Eaytoy):

[mediawiki/core@REL1_38] EditPage: make most methods private

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

Change 773856 merged by jenkins-bot:

[mediawiki/core@REL1_38] EditPage: make most methods private

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

Additional things that can be made private (or removed) (deployed uses refers to things outside of core):

EditPage::$editFormTextAfterWarn - NO deployed uses, can remove entirely
EditPage::$editFormTextBottom - NO deployed uses, can remove entirely

EditPage::setHeaders() - I *think* there are no deployed uses, but given how many other methods by this name exist I'm not positive
EditPage::showEditForm() - NO deployed uses
EditPage::showDiff() - NO deployed uses
EditPage::getPreviewText() - NO deployed uses
EditPage::getEditButtons() - NO deployed uses
EditPage::spamPageWithContent() - NO deployed uses


Also,
EditPage::getCheckboxesWidget() - can be made protected (overridden by ProofreadPage)

@Daimona care to continue working on this?

Those with no deployed uses but that do have 3rd party uses should go through the deprecation process. We can backport the deprecation to 1.38 and remove them on master though :)

Change 773996 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Remove some deprecated stuff from EditPage

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

Change 773996 merged by jenkins-bot:

[mediawiki/core@master] EditPage: Remove various deprecated properties and methods

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

Change #1113211 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/core@master] editpage: Remove deprecated EditPage::internalAttemptSave()

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

Change #1113211 merged by jenkins-bot:

[mediawiki/core@master] editpage: Remove deprecated EditPage::internalAttemptSave()

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

matmarex assigned this task to DannyS712.
matmarex subscribed.

All of the methods and properties listed in the task description have been made private or removed. There is more that could be done, but I think you can claim the credit for the work done so far, and this task can be closed.

Change #1138139 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] editpage: Rename internalAttemptSavePrivate back to internalAttemptSave

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

Change #1138139 merged by jenkins-bot:

[mediawiki/core@master] editpage: Rename internalAttemptSavePrivate back to internalAttemptSave

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