Page MenuHomePhabricator
Create Task
Maniphest T104055

ContentTranslation fails to adapt links with spaces
Closed, ResolvedPublic
Actions

Description

Links with spaces are not adapted.

To reproduce:

  • Translate Róisín Murphy from English to Hebrew.
  • In the "Early life" section in the source column, click the "Roman Catholic" link.

Observed: the link is not adapted, even though https://en.wikipedia.org/wiki/Catholic_Church has a corresponding article in he.wikipedia.

In the fetchLinkPairs function, the link titles are cached with underscores ('Catholic_Church'), but they are read with spaces ('Catholic Church').

This is a regression, I remember fixing this months ago.

Details

SubjectRepoBranchLines +/-
Consistently use title.toText() for cache accessmediawiki/extensions/ContentTranslationmaster+6 -6
Consistently use title.toText() for cache accessmediawiki/extensions/ContentTranslationwmf/1.26wmf11+6 -6
Customize query in gerrit

Related Objects
Search...

Event Timeline

Amire80 created this task.Jun 27 2015, 9:46 AM
Amire80 raised the priority of this task from to High.
Amire80 updated the task description. (Show Details)
Amire80 added a project: ContentTranslation.
Amire80 added a subscriber: Amire80.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 27 2015, 9:46 AM
Amire80 added a comment.Jun 27 2015, 9:47 AM

This happens both in production and in http://he.wikipedia.beta.wmflabs.org/ .

Amire80 mentioned this in T104066: ContentTranslation's postMT hook is fired twice even though no MT is done.Jun 27 2015, 2:00 PM
Amire80 renamed this task from ContentTranslation fails to adapt some links to ContentTranslation fails to adapt links with spaces.Jun 27 2015, 4:07 PM
Amire80 updated the task description. (Show Details)
Amire80 set Security to None.
Amire80 assigned this task to santhosh.Jun 28 2015, 8:50 AM
Amire80 added a subscriber: santhosh.
Nikerabbit updated the task description. (Show Details)Jun 28 2015, 9:04 AM
Nikerabbit added a subscriber: Nikerabbit.Jun 28 2015, 10:22 AM

The cause for this seems to be that the code accesses the global cache.linkPairs variable directly. This means that some code call title.toString directly while other code relies on stored title as a text which is in different format.

My recommendation is to make linkPair fetching encapsulated in it's own class which can be instantiated as an object with getters that perform the appropriate normalization. This requires some refactoring so that the current users of the global variable can handle promises instead. This will also solve race conditions if there are any, because the getters can check that the data is loaded before accessed.

Diffusion mentioned this in rMEXT375afb9f8bef: Updated mediawiki/extensions Project: mediawiki/extensions/ContentTranslation….Jun 29 2015, 3:35 AM
Amire80 mentioned this in rECTX84a1fdcd545a: Don't try to fetch link pairs if there are no links.Jun 29 2015, 3:36 AM
santhosh closed subtask T104124: Link adaptation does unnecessary API queries with zero items as Resolved.Jun 29 2015, 4:06 AM
gerritbot added a subscriber: gerritbot.Jun 29 2015, 5:07 AM

Change 221568 had a related patch set uploaded (by Santhosh):
Consistently use title.toText() for cache access

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

gerritbot added a project: Patch-For-Review.Jun 29 2015, 5:07 AM
gerritbot added a comment.Jun 29 2015, 10:49 AM

Change 221568 merged by jenkins-bot:
Consistently use title.toText() for cache access

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

Amire80 mentioned this in rECTXd232fccb2a8a: Consistently use title.toText() for cache access.Jun 29 2015, 10:49 AM
santhosh mentioned this in rMEXT2a552f894302: Updated mediawiki/extensions Project: mediawiki/extensions/ContentTranslation….Jun 29 2015, 10:49 AM
gerritbot added a comment.Jun 29 2015, 12:09 PM

Change 221605 had a related patch set uploaded (by KartikMistry):
Consistently use title.toText() for cache access

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

gerritbot added a comment.Jun 29 2015, 12:13 PM

Change 221605 merged by jenkins-bot:
Consistently use title.toText() for cache access

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

KartikMistry mentioned this in rECTXc08ca7b13894: Consistently use title.toText() for cache access.Jun 29 2015, 12:13 PM
Diffusion mentioned this in rMW3f51f8d3cc3e: Updated mediawiki/core Project: mediawiki/extensions/ContentTranslation….Jun 29 2015, 12:14 PM
Nikerabbit closed subtask T104122: Two getLink functions in same file is confusing as Resolved.Jun 29 2015, 7:38 PM
Amire80 closed subtask T104123: Use more appropriate title normalization in the link tool as Resolved.Jul 2 2015, 4:40 PM
Amire80 moved this task from Needs Triage to CX6 on the ContentTranslation board.
Amire80 edited projects, added ContentTranslation-Release6; removed Patch-For-Review.Jul 2 2015, 4:42 PM

Is T104126 an actual bug or just technical debt?

Amire80 moved this task from Backlog to Links on the ContentTranslation-Release6 board.Jul 2 2015, 9:37 PM
santhosh closed this task as Resolved.Jul 6 2015, 11:13 AM
santhosh removed a subtask: T104126: getLink in linktools abuses the global cache variable.

T104126 is tech debt

Arrbee added a project: Maintenance-Worktype.Sep 7 2015, 11:46 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL