Page MenuHomePhabricator

The SidebarTranslate gadget is broken when used with Compact Language Links
Closed, ResolvedPublic

Description

The SidebarTranslate gadget is broken when used with Compact Language Links.

A JS error is shown: Cannot read property 'toLowerCase' of null TypeError: Cannot read property 'toLowerCase' of null

This happens because the gadget adds more <a> elements, and when the ULS code is trying to process all the <a> elements, it fails when it doesn't find an element with a lang attribute.

Several possible solutions:

  • Add a particular class to the <a> element of the actual interlanguage link, so that ULS would only load that element. This will have to be in core MediaWiki.
  • When querying for elements, use this.$interlanguageList.find( 'li.interlanguage-link > a:first-child' ) instead of just this.$interlanguageList.find( 'li.interlanguage-link > a' ). This should solve the problem with this gadget, but may break with other gadgets that add <a> elements.

Event Timeline

Amire80 created this task.May 16 2016, 12:19 PM
Restricted Application added subscribers: Zppix, Aklapper. · View Herald Transcript
Amire80 triaged this task as High priority.May 16 2016, 12:22 PM
Amire80 moved this task from Backlog to Gargets and local customization on the ULS-CompactLinks board.
Amire80 renamed this task from The SidebarTranslate gadget overrides Compact Language Links to The SidebarTranslate gadget is broken when used with Compact Language Links.Sep 10 2016, 1:46 PM
Amire80 updated the task description. (Show Details)

Change 309733 had a related patch set uploaded (by Amire80):
Query only for <a> elements that are interlanguage links targets

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

Change 309734 had a related patch set uploaded (by Amire80):
Query for <a> elements with first-child

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

I committed patches for both suggestions. Only one should be merged.

https://gerrit.wikimedia.org/r/309733 is the one I prefer because it's more robust and the core change can be useful for other purposes, but if that core change is not desirable, then https://gerrit.wikimedia.org/r/309734 should work.

Yet another solution would be fix the particular gadget, of course, but other gadgets can cause similar breakage, so a solution inside the extension code should be more comprehensive.

See some more information at https://en.wikipedia.org/wiki/MediaWiki_talk:Gadget-SidebarTranslate.js .

By applying the suggestions on that page, the bug will be fully resolved, although in a hacky way. Reviewing and merging the Gerrit patches mentioned above would be more robust, but both things should be done anyway.

Thanks.

After waiting for a few days, I went bold and applied the changes in the English Wikipedia. If anything breaks, blame me :)

I'll still be happy for a review of the patches for core and ULS.

Elitre added a subscriber: Elitre.Sep 14 2016, 11:56 AM

Change 309734 abandoned by Amire80:
Query for <a> elements with first-child

Reason:
Not needed now that I06c80945af785477be52096022c8493e7f82c298 is merged.

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

Amire80 lowered the priority of this task from High to Low.Oct 3 2016, 9:51 AM

Changing priority to Low. The current version of the gadget is already functional along with Compact Language Links. The remaining fixes will be useful in general, but not essential for this particular bug.

Change 309733 merged by jenkins-bot:
Query only for <a> elements that are interlanguage links targets

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

Amire80 closed this task as Resolved.Nov 7 2016, 9:49 AM
Amire80 claimed this task.

All the patches are merged and deployed and the gadget works. Thanks for the help.

Amire80 moved this task from QA to Done on the Language-Q2-2016-17 Sprint 2 board.Nov 7 2016, 9:49 AM