Page MenuHomePhabricator

[Bug] [object Object] appears in TalkOverlay
Closed, ResolvedPublic

Description

Steps to reproduce

  1. Login to english wikipedia
  2. Visit https://en.m.wikipedia.org/w/index.php?title=Main_Page#/talk/19
  3. Notice "[object Object] [object Object]" appears instead of readable text

talk-overlay.png (1×826 px, 100 KB)

Expected results

  • Readable text appears

Actual results

  • "[object Object] [object Object]" appears

Environments observed

  • Browser version: Chrome 74.0.3729.157
  • OS version: Mojave 10.14.15
  • Device model: MacBook Pro
  • Device language: en

QA Steps:

  • Go to mobile version on staging - https://reading-web-staging.wmflabs.org/wiki/Main_Page
  • Click "Discussion" button on bottom of the page (you have to be logged in)
  • There are 2 discussion available, check that when you view those dicussions, no [object Object] appears in the TalkOverlay (it was visible on the bottom of the talk overlay)
  • Additionally, please smoke tests most important mobile frontend features. The change was done in the templates rendering logic, which is widely used across whole mobile website, thus we should verify that mobile site is still usuable (login/logout/contributions/recent changes/talk/watchlist etc)

Developer notes

This seems to stem from our removal of Hogan in T220620 (https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/MobileFrontend/+/506337/). A line in our PageGateaway code (used by TalkOverlay) [[ https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/047c883f2550e54ca856d741bef91be3eaac8437/src/mobile.startup/PageGateway.js#L80 | depended on the return type of .render to be a string ]]. However, our new code used mediawiki.template.mustache (a wrapper around mustache) whose .render method returned a JQuery object instead. It does not affect every talk overlay, but it does affect ones with wikitext that has multiple sections.

QA Results

Event Timeline

nray updated the task description. (Show Details)
nray updated the task description. (Show Details)

Change 510971 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/extensions/MobileFrontend@master] 🐛 Bug: Fix util.js template API

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

Change 510971 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] 🐛 Bug: Fix util.js template API

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

Edtadros added a subscriber: Edtadros.

Test Result

Status: ✅ PASS
OS: macOS Mojave
Browser: Chrome
Device: MBP
Emulated Device: iPhoneX

Test Artifact(s):

QA Steps:

Go to mobile version on staging - https://reading-web-staging.wmflabs.org/wiki/Main_Page
Click "Discussion" button on bottom of the page (you have to be logged in)
There are 2 discussion available, check that when you view those dicussions, no [object Object] appears in the TalkOverlay (it was visible on the bottom of the talk overlay)

✅ AC1: AMC OFF / Beta OFF:

T223927-AmcOffBetaOff-2.png (16×1 px, 2 MB)

T223927-AmcOffBetaOff-1.png (9×1 px, 1 MB)

T223927-AmcOffBetaOff.png (2×1 px, 116 KB)

✅ AC2: AMC ON / Beta OFF:

T223927-AmcOnBetaOff-2.png (16×1 px, 3 MB)

T223927-AmcOnBetaOff-1.png (9×1 px, 1 MB)

T223927-AmcOnBetaOff.png (2×1 px, 116 KB)

✅ AC3: AMC ON / Beta ON:

T223927-AmcOnBetaOn-2.png (16×1 px, 3 MB)

T223927-AmcOnBetaOn-1.png (9×1 px, 1 MB)

T223927-AmcOnBetaOn.png (2×1 px, 116 KB)

✅ AC4: AMC OFF / Beta ON:

T223927-AmcOffBetaOn-2.png (16×1 px, 3 MB)

T223927-AmcOffBetaOn-1.png (9×1 px, 1 MB)

T223927-AmcOffBetaOn.png (2×1 px, 116 KB)

✅ AC5: Additionally, please smoke tests most important mobile frontend features. The change was done in the templates rendering logic, which is widely used across whole mobile website, thus we should verify that mobile site is still usuable (login/logout/contributions/recent changes/talk/watchlist etc)

I didn't see any issues with any of the pages/functions listed above.

@ovasileva Should this one be assigned the Audiences-QA (RW-Test-Cases) tag?

@ovasileva Should this one be assigned the Audiences-QA (RW-Test-Cases) tag?

Since it's a bug, I think we can go ahead without the tag. Resolving.