Page MenuHomePhabricator

MobileFormatter should not be responsible for adding edit links (duplicate code paths)
Closed, ResolvedPublic2 Estimate Story Points

Description

When enabling live Wikipedia data on reading web staging I noticed that the edit link was behaving strangely in Vector. After some investigation there is a bug. Fixing it will make this problem go away :)

In MinervaNeue the method doEditSectionLink is supposed to create an edit link for all headings

In MobileFrontend the MobileFormatter removes any elements which match .mw-editsection. Confusingly it also adds a toggling indicator to headings to avoid a flash of unstyled content. The former is a noop, as edit section links are handled by Skin::doEditSectionLink. The latter is badly documented.

The MobileFrontend mw-editsection code should be removed and the toggling indicator documented better.

Details

Related Gerrit Patches:
mediawiki/extensions/MobileFrontend : masterSkinMinerva should deal with edit section link not MobileFormatter

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 24 2017, 8:06 PM

Change 367477 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] SkinMinerva should deal with edit section link not MobileFormatter

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

Jdlrobson updated the task description. (Show Details)Jul 24 2017, 8:07 PM
Jdlrobson renamed this task from Is MobileFormatter or skin responsible for adding edit links? to MobileFormatter should not be responsible for adding edit links.Jul 24 2017, 8:20 PM
Jdlrobson renamed this task from MobileFormatter should not be responsible for adding edit links to MobileFormatter should not be responsible for adding edit links (duplicate code paths).
Jdlrobson updated the task description. (Show Details)
Jdlrobson edited projects, added MinervaNeue (Desktop); removed MediaWiki-Parser.
Jdlrobson updated the task description. (Show Details)Jul 24 2017, 11:16 PM
pmiazga added a subscriber: pmiazga.

Pulling into the current sprint for visibility.

Jdlrobson triaged this task as Medium priority.Jul 25 2017, 6:37 PM

Code debt/duplication should always be removed to avoid confusion but there are no known bugs for this to be high priority.

This task requires estimation. I vote: 2 as it requires changes both in MobileFrontend extension and MinervaNeue skin with proper coordination (both changes have to go at once).

This comment was removed by pmiazga.

No change in MinervaNeue needed.. What am i missing?

No? My understanding was that we move some logic from MobileFrontend to Minerva.

Jdlrobson added a comment.EditedJul 25 2017, 8:56 PM

Nope the logic is already there. That's the bug. We've duplicated logic in mobilefrontend unnecessarily :)

MBinder_WMF set the point value for this task to 2.Jul 26 2017, 5:21 PM

Change 367477 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] SkinMinerva should deal with edit section link not MobileFormatter

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

Jdlrobson updated the task description. (Show Details)Jul 27 2017, 10:41 PM
Jdlrobson closed this task as Resolved.Jul 28 2017, 4:57 PM

This is a technical task.