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

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)

This also happens on all pages in the desktop version of the skin: https://en.wikipedia.org/w/index.php?title=Heading&useskin=minerva
It happens on the ?action=edit page (https://en.m.wikipedia.org/wiki/Columbia_River?action=edit)

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:

Details

	Subject	Repo	Branch	Lines +/-
	Use lead section when editing page without section marks	mediawiki/skins/MinervaNeue	master	+4 -1
	Make sure lead section exists before accessing its method	mediawiki/skins/MinervaNeue	master	+4 -3

Customize query in gerrit

Related Objects

Mentioned In: T171488: Desktop Minerva accidentally enables section collapsing on certain sections
T169003: Desktop Minerva initialization problems due to MobileFormatter not executing
T171062: Unknown dependency: mobile.site when trying to use Minerva on desktop
T166005: Editing/section collapsing edge cases
Mentioned Here: T172948: Desktop Minerva main edit icon sometimes points to section 1

Event Timeline

Jdlrobson created this task.Feb 13 2017, 7:29 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 13 2017, 7:29 PM

Jdforrester-WMF triaged this task as High priority.Feb 14 2017, 8:03 PM

Jdforrester-WMF moved this task from To Triage to External and Administrivia on the VisualEditor board.

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 mentioned this in T166005: Editing/section collapsing edge cases.Jul 13 2017, 5:46 PM

Jdlrobson updated the task description. (Show Details)Jul 18 2017, 5:22 PM

Jdlrobson updated the task description. (Show Details)

Jdlrobson added a project: Web-Team-Backlog.

@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 mentioned this in T171062: Unknown dependency: mobile.site when trying to use Minerva on desktop.Jul 19 2017, 4:25 PM

Jdlrobson added a project: Readers-Web-Kanbanana-Board-Old.Jul 24 2017, 4:37 PM

Jdlrobson updated the task description. (Show Details)Jul 25 2017, 4:28 PM

Jdlrobson updated the task description. (Show Details)

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

Jdlrobson moved this task from Upcoming to 2017-18 Q1 on the Web-Team-Backlog board.Jul 25 2017, 5:28 PM

Jdlrobson moved this task from To Do to Needs Design Review on the Readers-Web-Kanbanana-Board-Old board.Jul 26 2017, 5:52 PM

• bmansurov claimed this task.Jul 31 2017, 7:02 PM

• bmansurov moved this task from Needs Design Review to Doing on the Readers-Web-Kanbanana-Board-Old 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

gerritbot added a project: Patch-For-Review.Jul 31 2017, 7:21 PM

• bmansurov moved this task from Doing to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.Jul 31 2017, 7:21 PM

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

pmiazga moved this task from Needs Code Review to Needs QA on the Readers-Web-Kanbanana-Board-Old board.Jul 31 2017, 9:29 PM

Jdlrobson moved this task from Needs QA to Ready for Signoff on the Readers-Web-Kanbanana-Board-Old board.Jul 31 2017, 9:29 PM

Jdlrobson moved this task from Ready for Signoff to Needs QA on the Readers-Web-Kanbanana-Board-Old board.

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 reassigned this task from • bmansurov to ABorbaWMF.Jul 31 2017, 9:32 PM

Jdlrobson updated the task description. (Show Details)

Jdlrobson added a subscriber: • bmansurov.

ReleaseTaggerBot added a project: MW-1.30-release-notes (WMF-deploy-2017-08-01_(1.30.0-wmf.12)).Jul 31 2017, 10:00 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.

• bmansurov claimed this task.Aug 1 2017, 9:53 PM

• bmansurov moved this task from Needs More Work to Doing on the Readers-Web-Kanbanana-Board-Old 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

gerritbot added a project: Patch-For-Review.Aug 1 2017, 10:18 PM

• bmansurov moved this task from Doing to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.Aug 1 2017, 10:19 PM

Jdlrobson moved this task from Needs Code Review to Ready for Signoff on the Readers-Web-Kanbanana-Board-Old board.Aug 2 2017, 2:36 PM

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

ReleaseTaggerBot edited projects, added MW-1.30-release-notes (WMF-deploy-2017-08-08_(1.30.0-wmf.13)); removed MW-1.30-release-notes (WMF-deploy-2017-08-01_(1.30.0-wmf.12)).Aug 2 2017, 3:00 PM

• 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

In T157995#3490921, @Jdlrobson wrote:

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.

It happens on the ?action=edit page (https://en.m.wikipedia.org/wiki/Columbia_River?action=edit)

Fixed (tested on the Beta Cluster) 👍

This also happens on all pages in the desktop version of the skin: https://en.wikipedia.org/w/index.php?title=Heading&useskin=minerva

Fixed (tested on the Beta Cluster) 👍

Per T157995#3493997.

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 claimed this task.Aug 2 2017, 10:14 PM

• 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

• bmansurov moved this task from Doing to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.Aug 3 2017, 3:24 PM

Jdlrobson mentioned this in T169003: Desktop Minerva initialization problems due to MobileFormatter not executing.Aug 4 2017, 2:46 PM

Jdlrobson mentioned this in T171488: Desktop Minerva accidentally enables section collapsing on certain sections.

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

Jdlrobson moved this task from Needs Code Review to Needs More Work on the Readers-Web-Kanbanana-Board-Old board.Aug 8 2017, 7:08 PM

• bmansurov moved this task from Needs More Work to Doing on the Readers-Web-Kanbanana-Board-Old board.Aug 8 2017, 7:41 PM

• bmansurov moved this task from Doing to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.Aug 8 2017, 8:37 PM

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

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

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

• Petar.petkovic removed a project: Patch-For-Review.Feb 23 2019, 11:52 AM

• Petar.petkovic removed a subscriber: gerritbot.

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

Description

Developer notes

Test plan

Details

Related Objects

Event Timeline

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