Page MenuHomePhabricator
Create Task
Maniphest T171519

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

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

SubjectRepoBranchLines +/-
SkinMinerva should deal with edit section link not MobileFormattermediawiki/extensions/MobileFrontendmaster+1 -2
Customize query in gerrit

Related Objects

Event Timeline

Jdlrobson created this task.Jul 24 2017, 8:06 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 24 2017, 8:06 PM
gerritbot subscribed.Jul 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

gerritbot added a project: Patch-For-Review.Jul 24 2017, 8:06 PM
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 added a project: Technical-Debt (RW-Tech-Debt).
Jdlrobson updated the task description. (Show Details)Jul 24 2017, 11:16 PM
Jdlrobson moved this task from Needs Prioritization to Upcoming on the Web-Team-Backlog board.Jul 25 2017, 12:20 AM
pmiazga added projects: Readers-Web-Kanbanana-Board-Old, Unplanned-Sprint-Work.EditedJul 25 2017, 2:16 PM
pmiazga subscribed.

Pulling into the current sprint for visibility.

pmiazga moved this task from To Do to Needs More Work on the Readers-Web-Kanbanana-Board-Old board.Jul 25 2017, 2:18 PM
Jdlrobson moved this task from Needs More Work to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.Jul 25 2017, 4:51 PM
Jdlrobson moved this task from Upcoming to 2017-18 Q1 on the Web-Team-Backlog board.Jul 25 2017, 5:28 PM
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.

pmiazga added a comment.Jul 25 2017, 8:35 PM

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).

pmiazga added a comment.Jul 25 2017, 8:35 PM
This comment was removed by pmiazga.
Jdlrobson added a comment.Jul 25 2017, 8:43 PM

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

pmiazga added a comment.Jul 25 2017, 8:51 PM

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
Jdlrobson claimed this task.Jul 26 2017, 6:13 PM

https://gerrit.wikimedia.org/r/367477 has a +1 from Timo.

Jdlrobson moved this task from Needs Code Review to Needs More Work on the Readers-Web-Kanbanana-Board-Old board.Jul 27 2017, 9:37 PM
gerritbot added a comment.Jul 27 2017, 10:30 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

pmiazga moved this task from Needs More Work to Ready for Signoff on the Readers-Web-Kanbanana-Board-Old board.Jul 27 2017, 10:34 PM
pmiazga removed a project: Patch-For-Review.
Jdlrobson updated the task description. (Show Details)Jul 27 2017, 10:41 PM
ReleaseTaggerBot added a project: MW-1.30-release-notes (WMF-deploy-2017-08-01_(1.30.0-wmf.12)).Jul 27 2017, 11:00 PM
Jdlrobson mentioned this in T169823: Design changes to desktop print styles .Jul 28 2017, 4:40 PM
Jdlrobson closed this task as Resolved.Jul 28 2017, 4:57 PM

This is a technical task.

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL