Page MenuHomePhabricator

Clicking [ or ] surrounding a reference throws an error when trying to present the reference drawer
Closed, ResolvedPublic

Description

When you click the [ or ] of a reference you get an error, because the click handler assumes that you clicked the link, not the surrounding brackets

function showReference( ev, drawer, page ) {
		var urlComponents,
			$dest = $( ev.target ),
			href = $dest.attr( 'href' );

		ev.preventDefault();

		// If necessary strip the URL portion of the href so we are left with the
		// fragment
		urlComponents = href.split( '#' );

^^ ev.target will be text node '[ '. causing href var to be undefined|null causing an error when trying to .split() it.

Replication

*See JS error

Event Timeline

TheDJ created this task.May 30 2017, 10:09 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 30 2017, 10:09 AM
Jdlrobson added a subscriber: Jdlrobson.

Seems like a 1 pointer, so adding to sprint.

Jdlrobson triaged this task as Normal priority.May 30 2017, 7:10 PM
Jdlrobson added a comment.EditedMay 30 2017, 7:12 PM

@TheDJ can you give a page/browser and example? all the '[' I see are children of the link. I can't seem to replicate this.

TheDJ added a comment.May 30 2017, 8:31 PM

See betalabs

<a href="#cite_note-Kirkland_geol_pp.21.E2.80.9350-58"><span>[</span>58<span>]</span></a>

The target will be the span, instead of the parent a element. This is because beta has a different configuration for rendering that element:
https://en.wikipedia.beta.wmflabs.org/wiki/MediaWiki:Cite_reference_link

Not sure how many wiki's in production have non-standard configs like that...

Jdlrobson updated the task description. (Show Details)May 30 2017, 8:58 PM

Change 356301 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Use currentTarget rather than target in ref link click handler

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

Change 356301 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Use currentTarget rather than target in ref link click handler

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

Jdlrobson reassigned this task from Jdlrobson to ABorbaWMF.May 31 2017, 4:53 PM

I'm pretty confident about this change, but can you do a quick review of functionality relating to references to check this hasn't broken anything related?

I did a quick review using an iphone 7+, a Pixel, and a Nexus 4

I used the dog article on beta: https://en.m.wikipedia.beta.wmflabs.org/wiki/Dog

Clicked through the references. Everything worked.

Jdlrobson closed this task as Resolved.Jun 1 2017, 10:45 PM

thank you!