Title not shown for page named "0" on mobile
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Nloveladyallen
	Apr 16 2017, 9:31 PM

Description

In the mobile site of any Wikipedia (*.m.wikipedia.org) the article title is not displayed if it is "0".

Examples: https://en.m.wikipedia.org/wiki/0, https://test2.m.wikipedia.org/wiki/0

Details

	Subject	Repo	Branch	Lines +/-
	Show title on the page named '0'	mediawiki/skins/MinervaNeue	master	+2 -1

Customize query in gerrit

Event Timeline

Nloveladyallen created this task.Apr 16 2017, 9:31 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 16 2017, 9:31 PM

Here's a screenshot of what the task is talking about.
On left, there is no Article Title "Heading" shown (en.m.wikipedia.org/wiki/0). On right, there is an Article Title "Heading" shown (en.m.wikipedia.org/wiki/Human)

Jdlrobson added projects: MobileFrontend, Web-Team-Backlog, MinervaNeue.Jul 19 2017, 1:11 AM

Jdlrobson moved this task from Needs triage to MobileFrontend specific on the Mobile board.

Jdlrobson triaged this task as Low priority.Jul 19 2017, 3:37 PM

Jdlrobson moved this task from Incoming to Triaged but Future on the Web-Team-Backlog board.

Jdlrobson added projects: good first task, patch-welcome.

Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptJul 19 2017, 3:37 PM

xSavitar subscribed.Jul 19 2017, 10:34 PM

xSavitar awarded a token.Jul 19 2017, 11:44 PM

Jdlrobson moved this task from Backlog to Bugs on the MinervaNeue board.Jul 20 2017, 10:47 PM

xSavitar added a project: Africa-Wikimedia-Developers.Jul 22 2017, 8:45 AM

xSavitar moved this task from Backlog to New Contributor's Tasks (Incoming) on the Africa-Wikimedia-Developers board.

Typical php pitfall:

https://secure.php.net/manual/en/language.types.boolean.php

When converting to boolean, the following values are considered FALSE: [...] the empty string, and the string "0"

Looing at SkinMinerva::getHeadingHtml:

$pageTitle = $this->getOutput()->getPageTitle();
if ( $pageTitle ) {
	$heading = $pageTitle;
}

I haven't sent this in as a patch because I'm not sure why the if( $pageTitle ) was added in the first place - hence I don't know whether removing it might be harmful to some other edge case. Should we go with just removing the if or exclude this edge case with if( $pageTitle || $pageTitle === '0' )?

@EddieGP
Vector also expects that title's might not be present.

// Loose comparison with '!=' is intentional, to catch null and false too, but not '0'
                        if ( $this->data['title'] != '' ) {
                        ?>
                        <h1 id="firstHeading" class="firstHeading" lang="<?php $this->text( 'pageLanguage' ); ?>"><?php
                                 $this->html( 'title' )
                        ?></h1>
                        <?php
                        } ?>

Change 368585 had a related patch set uploaded (by EddieGP; owner: EddieGP):
[mediawiki/skins/MinervaNeue@master] Show title on the page named '0'

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

gerritbot added a project: Patch-For-Review.Jul 30 2017, 5:50 PM

Thanks @TheDJ, that snippet (especially with the comment given there) convinced me that this check exists on purpose. ;-) Hence I've used the same comparison as Vector did (and re-used the comment from there, sums it up quite nicely).

Bringing this patch as unplanned sprint work to our current sprint. @EddieGP wrote fix and it works nicely. @ovasileva could you verify and signoff this task?