Impossible to edit pages without sections on Minerva skin (including all pages in desktop Minerva)
Closed, ResolvedPublic2 Story Points

Description

As a logged in user when I try to edit https://en.m.wikipedia.org/wiki/User:Jdlrobson/common.js I get:

Exception in module-execute in module skins.minerva.editor:
load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=minerva&version=1rfuz0b:176 TypeError: Cannot read property 'text' of null TypeError: Cannot read property 'text' of null
    at setupEditor (eval at <anonymous> (load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=minerva&version=1rfuz0b:4), <anonymous>:182:97)
    at init (eval at <anonymous> (load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=minerva&version=1rfuz0b:4), <anonymous>:182:369)
    at eval (eval at <anonymous> (load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=minerva&version=1rfuz0b:4), <anonymous>:183:576)
    at mw.loader.implement.mobile-frontend-editor-disabled (eval at <anonymous> (load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=minerva&version=1rfuz0b:4), <anonymous>:183:586)
    at Object.<anonymous> (load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=minerva&version=1rfuz0b:161)
    at fire (load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=minerva&version=1rfuz0b:45)
    at Object.add [as done] (load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=minerva&version=1rfuz0b:45)
    at Object.always (load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=minerva&version=1rfuz0b:46)
    at runScript (load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=minerva&version=1rfuz0b:161)
    at checkCssHandles (load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=minerva&version=1rfuz0b:162)

Developer notes

Page::getSection assumes the MobileFormatter has been run and looks for the lead section. However on pages which have not been formatted (e.g. pages that are not wikitext such as MediaWiki:Common.js) or desktop pages this blows up.

We'll add a unit test and make sure the editor opens without the section parameter in this case

Test plan

Testing in one browser should be sufficient.

Visit the following URLs and verify that clicking the edit icon works as expected and no errors are thrown in the JS Developer console:

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 13 2017, 7:29 PM
Jdforrester-WMF triaged this task as High priority.
Jdlrobson renamed this task from Impossible to edit certain pages on mobile site to Impossible to edit pages without sections on Minerva skin (including all pages in desktop Minerva).Jul 13 2017, 5:35 PM
Jdlrobson edited projects, added MinervaNeue (Desktop); removed MobileFrontend.
Jdlrobson updated the task description. (Show Details)
Jdlrobson updated the task description. (Show Details)Jul 18 2017, 5:22 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson added a project: Readers-Web-Backlog.
Jdlrobson added a subscriber: ovasileva.

@ovasileva this is a 1 or 2 pointer and doesn't look like a priority for editing.. so should we fix it?

@Jdlrobson - yup. Pulling into upcoming. Let's estimate today.

Jdlrobson updated the task description. (Show Details)Jul 25 2017, 4:28 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson set the point value for this task to 2.Jul 25 2017, 4:32 PM

Well defined bug - can be captured in a test and update to when selector matches no elements.

bmansurov moved this task from To Do to Doing on the Readers-Web-Kanban-Board board.

Change 368834 had a related patch set uploaded (by Bmansurov; owner: Bmansurov):
[mediawiki/skins/MinervaNeue@master] Make sure lead section exists before accessing its method

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

pmiazga added a subscriber: pmiazga.

Tested locally, on master branch I was able to reproduce this error, after downloading patch error is gone. Moving to Needs QA.

Change 368834 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Make sure lead section exists before accessing its method

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

Jdlrobson updated the task description. (Show Details)Jul 31 2017, 9:32 PM
Jdlrobson reassigned this task from bmansurov to ABorbaWMF.
Jdlrobson added a subscriber: bmansurov.

There is an issue
Visiting https://en.wikipedia.beta.wmflabs.org/wiki/Template:Other_areas_of_Wikipedia?useskin=minerva when I click the edit link it tries to edit section 1 rather than section 0.

bmansurov claimed this task.Aug 1 2017, 9:53 PM
bmansurov moved this task from Needs More Work to Doing on the Readers-Web-Kanban-Board board.
bmansurov added a subscriber: ABorbaWMF.

Change 369565 had a related patch set uploaded (by Bmansurov; owner: Bmansurov):
[mediawiki/skins/MinervaNeue@master] Use lead section when editing page without section marks

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

Change 369565 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Use lead section when editing page without section marks

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

bmansurov removed bmansurov as the assignee of this task.Aug 2 2017, 4:49 PM
ovasileva assigned this task to phuedx.Aug 2 2017, 4:57 PM
phuedx added a comment.EditedAug 2 2017, 5:06 PM

There is an issue
Visiting https://en.wikipedia.beta.wmflabs.org/wiki/Template:Other_areas_of_Wikipedia?useskin=minerva when I click the edit link it tries to edit section 1 rather than section 0.

I still see this issue when on https://en.wikipedia.beta.wmflabs.org/wiki/Claude_Monet?useskin=minerva, for example.

I've double-checked and the changes are definitely deployed to the Beta Cluster.

phuedx added a comment.Aug 2 2017, 5:13 PM

Fixed (tested on the Beta Cluster) 👍

Fixed (tested on the Beta Cluster) 👍

I think you've discovered another issue. The task talks about pages without sections.

Page::getSection assumes the MobileFormatter has been run and looks for the lead section.

Desktop pages don't really have sections. They have section edit links. The MobileFormatter makes the sections and doesn't run on desktop. This is definitely in the scope of this task. It could have been worded better...

They have section edit links.

Yet they are marked as sections.

mw.mobileFrontend.getCurrentPage().getLeadSectionElement() returns the headings in Vector and sections in Minerva. I'm trying to make sense of the mumbo-jumbo code that we have.

bmansurov added a subscriber: phuedx.

Change 369934 had a related patch set uploaded (by Bmansurov; owner: Bmansurov):
[mediawiki/skins/MinervaNeue@master] Correctly identify the leadsection

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

Change 370087 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Page.js getLeadSectionElement returns page element when no sections located

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

I will create a new task capturing the remaining issue with the background of the discussion we've been having. The JS error has been addressed so sufficient progress has been made.

bmansurov removed bmansurov as the assignee of this task.Aug 9 2017, 6:14 PM
Jdlrobson closed this task as Resolved.Aug 9 2017, 8:32 PM
Jdlrobson claimed this task.

I have created T172948. Let's call this done.

Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptAug 9 2017, 8:32 PM