Page MenuHomePhabricator

Regression: Mobile talk page shows links in overlay header
Closed, ResolvedPublic2 Estimated Story Points

Description

If there's a wikilink in a talk page section heading, when that section is opened on mobile the link has display: block; in the overlay header. This makes the header taller than usual, and covers up the first part of the section's content.

Example with the heading Testing [[foobar]] wikilink
https://en.m.wikipedia.org/wiki/User_talk:The_wub/mobile_section_test#Testing_foobar_wikilink

Developer notes

See T243650#5839417

QA Steps

  • Visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Talk:Test_ascii_toc
  • There is a section titled "this is a title with bold text but it should not appear in overlay title" section. Notice that the "bold" word in that title is bolded. Click on this section.
  • After clicking the section, verify that the title in Overlay does not contain any bolded characters and that it appears okay (no weird spacing).

QA Results

ACStatusDetails
1T243650#5898221
2T243650#5898221

Event Timeline

Pcoombe created this task.Jan 25 2020, 12:24 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 25 2020, 12:24 AM
Jdlrobson added a project: Regression.
Jdlrobson added a subscriber: Jdlrobson.

This is a regression caused by T240487.
The behaviour was correct before but it needs to do that while avoiding the XSS that it tried to fix.
I dont think we should support HTML in overlay headers. Having a link or any styling in their is unnecessarily confusing.

I'd recommend using

			section: new mobile.Section( {
				line: $('<span>').text( $($headline).text() )[0].outerHTML,
				text: $heading.next().html()
			} ),
Jdlrobson renamed this task from Mobile talk page: display issues with wikilink in section heading to Regression: Mobile talk page shows links in overlay header.Jan 30 2020, 3:18 AM
Jdlrobson updated the task description. (Show Details)

Ah, okay. However I notice such links are visible but not clickable in the section title when it's collapsed, the action to open the overlay takes precedence. That makes sense, but if the link is removed from the overlay header then it will leave no way to open such a link at all.

Yeh section headings are pretty problematic on mobile due to the fact that clicking on one is what toggles open sections. I'd love to understand why they are used in place of a link/hatnote at the top of the section.

Jdlrobson triaged this task as Medium priority.Feb 3 2020, 1:59 PM
MBinder_WMF renamed this task from Regression: Mobile talk page shows links in overlay header to [S] Regression: Mobile talk page shows links in overlay header.Feb 12 2020, 5:49 PM
ovasileva renamed this task from [S] Regression: Mobile talk page shows links in overlay header to Regression: Mobile talk page shows links in overlay header.Feb 17 2020, 2:05 PM
ovasileva set the point value for this task to 2.

Change 572730 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Do not allow raw HTML in talk page overlay header

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

Fix pushed.

That makes sense, but if the link is removed from the overlay header then it will leave no way to open such a link at all.

The "read as wiki page" option still allows access to the link.

ovasileva lowered the priority of this task from Medium to Low.Feb 18 2020, 11:44 AM
ovasileva raised the priority of this task from Low to Medium.
nray assigned this task to Edtadros.EditedFeb 18 2020, 11:33 PM
nray added a subscriber: nray.

Patch has been +2'd, I just need to add QA steps which I'm doing now...

nray updated the task description. (Show Details)Feb 18 2020, 11:47 PM

Change 572730 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Do not allow raw HTML in talk page overlay header

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

Edtadros reassigned this task from Edtadros to ovasileva.Feb 19 2020, 6:15 PM
Edtadros added a subscriber: Edtadros.

Test Result

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

Test Artifact(s):

QA Steps

Visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Talk:Test_ascii_toc
✅ AC1: There is a section titled "this is a title with bold text but it should not appear in overlay title" section. Notice that the "bold" word in that title is bolded. Click on this section.

✅ AC2: After clicking the section, verify that the title in Overlay does not contain any bolded characters and that it appears okay (no weird spacing).

Edtadros updated the task description. (Show Details)Feb 19 2020, 6:16 PM
ovasileva closed this task as Resolved.Feb 20 2020, 12:11 PM