Page MenuHomePhabricator

Desktop table of contents JavaScript module is served in mobile skin
Closed, ResolvedPublic

Description

In the API we call $parserOutput->setTOCEnabled( false ); but for some reason we are no longer doing this for normal page views.
In onOutputPageParserOutput we call $outputPage->enableTOC( false ); which

Despite this, this request is not honoured. The table of contents is embedded in the page and wrapped in a toc element.

<toc><div id="toc" class="toc"><div id="toctitle"></div><h2>Contents</h2><div></div>
<ul><li class="toclevel-1 tocsection-1"><a href="#Background"><span class="tocnumber">1</span> <span class="toctext">Background</span></a>
<ul><li class="toclevel-2 tocsection-2"><a href="#Education"><span class="tocnumber">1.1</span> <span class="toctext">Education</span></a></li>
</ul></li>
<li class="toclevel-1 tocsection-3"><a href="#Career"><span class="tocnumber">2</span> <span class="toctext">Career</span></a>
<ul><li class="toclevel-2 tocsection-4"><a href="#Theatre"><span class="tocnumber">2.1</span> <span class="toctext">Theatre</span></a></li>
<li class="toclevel-2 tocsection-5"><a href="#Television"><span class="tocnumber">2.2</span> <span class="toctext">Television</span></a></li>
<li class="toclevel-2 tocsection-6"><a href="#Film"><span class="tocnumber">2.3</span> <span class="toctext">Film</span></a></li>
<li class="toclevel-2 tocsection-7"><a href="#Radio"><span class="tocnumber">2.4</span> <span class="toctext">Radio</span></a></li>
<li class="toclevel-2 tocsection-8"><a href="#Narration"><span class="tocnumber">2.5</span> <span class="toctext">Narration</span></a></li>
<li class="toclevel-2 tocsection-9"><a href="#Production_company"><span class="tocnumber">2.6</span> <span class="toctext">Production company</span></a></li>
</ul></li>
<li class="toclevel-1 tocsection-10"><a href="#Personal_life"><span class="tocnumber">3</span> <span class="toctext">Personal life</span></a>
<ul><li class="toclevel-2 tocsection-11"><a href="#In_the_media"><span class="tocnumber">3.1</span> <span class="toctext">In the media</span></a></li>
<li class="toclevel-2 tocsection-12"><a href="#Charity_and_social_action"><span class="tocnumber">3.2</span> <span class="toctext">Charity and social action</span></a></li>
</ul></li>
<li class="toclevel-1 tocsection-13"><a href="#Credits"><span class="tocnumber">4</span> <span class="toctext">Credits</span></a></li>
<li class="toclevel-1 tocsection-14"><a href="#See_also"><span class="tocnumber">5</span> <span class="toctext">See also</span></a></li>
<li class="toclevel-1 tocsection-15"><a href="#References"><span class="tocnumber">6</span> <span class="toctext">References</span></a></li>
<li class="toclevel-1 tocsection-16"><a href="#External_links"><span class="tocnumber">7</span> <span class="toctext">External links</span></a></li>
</ul></div>
</toc>

This causes big problems for mobile

  • The HTML is hidden but still there. We are also shipping a JSON of the table of contents. I hate duplication...
  • The table of contents also introduces JavaScript code that has not been tested on mobile and is not used.

We should either:

  • Use it to avoid the repaints with the table of contents JavaScript

OR

  • Remove it from the initial load.

In the short term we should do the latter.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 3 2016, 4:30 AM
Pcoombe added a subscriber: Pcoombe.Mar 3 2016, 5:41 PM
jhobs added a subscriber: jhobs.EditedMar 3 2016, 6:06 PM

I vote for option B, but no strong opinion either way.

Edit: We should probably get some preliminary data on % of page load.

@MaxSem do you know of any recent changes that may have broken OutputPage::enableTOC function?

Elitre added a subscriber: Elitre.Mar 3 2016, 10:39 PM
Florian added a subscriber: Florian.Mar 4 2016, 6:37 AM

Change 266337 had a related patch set uploaded (by Jdlrobson):
Fix parser cache issues

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

Okay I replicated this locally and can confirm this is the same bug as T124356.
I've posted 2 patches which fix this.

Change 275901 had a related patch set uploaded (by Jdlrobson):
Don't add mediawiki.toc to pages with toc stripped

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

Change 276009 had a related patch set uploaded (by Jdlrobson):
Fix parser cache issues

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

Change 266337 merged by jenkins-bot:
Fix parser cache issues

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

Change 276057 had a related patch set uploaded (by Jdlrobson):
Fix parser cache issues

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

Change 276009 merged by jenkins-bot:
Fix parser cache issues

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

Change 276057 merged by jenkins-bot:
Fix parser cache issues

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

https://gerrit.wikimedia.org/r/275901 is currently -1ed
This is proving tricky so some thoughts on how this can best be achieved would be appreciated.

Jdlrobson changed the task status from Open to Stalled.Mar 14 2016, 6:30 PM
Jdlrobson renamed this task from Desktop table of contents HTML is served in mobile views but not API to Desktop table of contents JavaScript module is served in mobile skin.Mar 16 2016, 9:50 PM
Jdlrobson moved this task from Backlog to Bugs on the MobileFrontend board.Mar 16 2016, 10:36 PM
Jdlrobson closed this task as Resolved.
Jdlrobson claimed this task.

Since HTML is fixed and this task was confusing people I've split the JavaScript issue into a subtask.