Page MenuHomePhabricator

Graph extension should exclude scripts tags so they will not be parsed by TextExtracts
Closed, DuplicatePublic

Description

Graph extension creates script nodes in its HTML output, which seem to get extracted as if they were text by Hovercards:

The graphs extension can avoid this by adding a class to the script nodes and making sure this is listed in the config option ExtractsRemoveClasses

Event Timeline

brion created this task.Jul 15 2015, 6:45 PM
brion updated the task description. (Show Details)
brion raised the priority of this task from to Needs Triage.
brion added projects: Graphs, Page-Previews.
brion added a subscriber: brion.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 15 2015, 6:45 PM

Indeed it looks like everything between parentheses has been stripped as if it were text.
rEPOPb587fcda2055: render.article: Remove text inside brackets along with the brackets

But if it isn't Graphs's fault, it's almost certainly TextExtracts'.

brion set Security to None.
TheDJ added a subscriber: TheDJ.Jul 16 2015, 8:21 PM

Only a problem if you don't have graphoid running right ?

brion added a comment.Jul 16 2015, 8:38 PM

Only a problem if you don't have graphoid running right ?

That bit of JS is actually the output for the graphoid-generated fallback image. :D Graph uses inline document.write and a noscript element to insert an img element if necessary (but not always when necessary). It's kind of scary.

Krinkle added a subscriber: Krinkle.Aug 5 2015, 1:44 AM

Only a problem if you don't have graphoid running right ?

That bit of JS is actually the output for the graphoid-generated fallback image. :D Graph uses inline document.write and a noscript element [..] kind of scary.

See also T107976.

Jdlrobson triaged this task as Low priority.Sep 18 2015, 7:58 PM
Jdlrobson added a subscriber: Jdlrobson.

How can I replicate this? Could you give me a sample article and api call?
Is this still a problem?

brion added a comment.Sep 18 2015, 8:02 PM

Steps to repro:

  • set up a wiki, install graph & popups
  • create a page that has a graph right at the beginning of the page
  • link to that page
  • hover cursor over the link

Thanks @brion is this all graphs generated or just with specific parameters. (Note to future bug fixer you will have to dig through https://www.mediawiki.org/wiki/Extension:Graph to see how to generate graph)

brion added a comment.Sep 18 2015, 9:28 PM

Should be any graph, as it's the JS preamble with the data trigger that's showing up. My test wiki at the time had a copy of one of the samples, I think the Adams chart: https://www.mediawiki.org/wiki/Extension:Graph/Demo#Graph_data_from_a_URL

Does this happen if RESTBase is used? If we switch to HTML previews this would go away.

Jdlrobson updated the task description. (Show Details)
Jdlrobson renamed this task from Hovercards can show JavaScript output by Graph extension to Graph extension should exclude scripts tags so they will not be parsed by TextExtracts.