Page MenuHomePhabricator

div block with read-more-container class shouldn't be rendered when/if not needed on Wikivoyage pages
Closed, ResolvedPublicBUG REPORT

Description

List of steps to reproduce (step by step, including full links if applicable):

What happens?:

  • With Chrome:
    • Sometimes (not always), is present a big empty space below the body content, but going up and down this empty space disappear.
  • With Firefox:
    • The empty space appear and then disappear on every reload

This empty space is originated by the following code:
<div class="read-more-container"></div>
that is located inside <div id="mw-data-after-content"></div>
but going up and down, the inner code disappear.

What should have happened instead?:
That space and the relevant code, shouldn't be there since the beginning.

The equivalent pages on Wikipedia do not have such behavior.

Software version (if not a Wikimedia wiki), browser information, screenshots, other information, etc:
Chrome 96.0.4664.45 (64 bit)
With Firefox 94.0.2 (64-bit)
However browsers should't be relevant. I suspect that there is a Javascript like ".read-more-container').remove()" that remove that block, but most likely that block shouldn't be there since the beginning.

Event Timeline

Andyrom75 updated the task description. (Show Details)
Andyrom75 renamed this task from read-more-container class shouldn't be rendered when/if not needed on it:voy pages to read-more-container class shouldn't be rendered when/if not needed on Wikivoyage pages.Dec 5 2021, 8:31 AM
Andyrom75 updated the task description. (Show Details)
Andyrom75 renamed this task from read-more-container class shouldn't be rendered when/if not needed on Wikivoyage pages to div block with read-more-container class shouldn't be rendered when/if not needed on Wikivoyage pages.Dec 5 2021, 9:39 AM
ovasileva triaged this task as Medium priority.Dec 6 2021, 4:09 PM
ovasileva added a project: Web-Team-Backlog.
Jdlrobson subscribed.

This is happen because Wikivoyage doesn't use the API for related articles recommendations. So one solution would be for wikivoyage to use the API so every page features related articles (in this situation the magic word overrides the API). If that's okay with the community that's a pretty simple configuration change.

To retain existing behaviour (without the API) we'd need to a server side check for whether the magic word has been used (by checking $out->getProperty( 'RelatedArticles' )) and to exclude the related articles element. This should be an easy check and could happen here [1].

[1] https://github.com/wikimedia/mediawiki-extensions-RelatedArticles/blob/master/includes/Hooks.php#L217

@Jdlrobson actually some pages use the "related features", like https://en.wikivoyage.org/wiki/Hawaii_Volcanoes_National_Park, but it seems that this mechanism is buggy.
If I reload that page whan I'm at the bottom I'll see the empty space, but if I pgup + pgdown, that spaces is filled with the related article. It's a kind of magic :-D

@Jdlrobson I've taken a look at the code you highlighted.
Although I'm not able to put my hands on that I have few questions:

  1. Is that code only for Wikivoyage or for any wiki-project?
  2. If only for Wikivoyage, does make sense to put the line $data .= \Html::element( 'div', [ 'class' => 'read-more-container' ] ); inside into an if-block where the condition is the presence of at least one related page? I suppose that $relatedPages[] at line 182 could help

Let me know

wgRelatedArticlesUseCirrusSearch is false only on wikivoyage wikis so provided your if block also checks that, yes your suggested solution makes sense.

If RelatedArticlesUseCirrusSearch is true, then that means articles can be pulled in via an API, so the read more container still needs to be rendered.

Hope this makes sense?

@Jdlrobson unfortunately I'm not enough expert to fully understand the implication of such configuration change, however, if you think that passing from "no API" to "API" will solve the issue and all the pages with 1 or more related articles, will keep on showing them (hence no side effect), well, yes, let's change the configuration for the whole project.

What do you think?

@Andyrom75 can you check in with the community?

This may give undesired results in certain cases, as the related articles API is based on similar text.

For example for https://it.wikivoyage.org/wiki/Spighe_Verdi the API returns the following:

Screen Shot 2021-12-20 at 9.56.25 AM.png (412×2 px, 72 KB)

https://en.wikivoyage.org/wiki/The_Most_Beautiful_Villages_of_France:

Screen Shot 2021-12-20 at 9.58.05 AM.png (360×2 px, 138 KB)

This can only be overriden by defining a related article.

@Jdlrobson so if I'm understanding it correctly, by default it will be shown certain article bases on "similar text", is it right?

However, since those articles can be overridden using "related" function, I think it's fine.

That said, I'll check it with a dedicated discussion.

Your understanding correctly. Thanks for the due diligence here. (I can't make any changes until January anyway as we have a deployment freeze right now)

@Jdlrobson I've started two discussions:

In both cases there is no objections on proceeding.

Feel free to apply the changes after January on your earliest convenience. In the meanwhile enjoy your Xmas holiday :-)

Change 752751 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[operations/mediawiki-config@master] Enable CirrusSearch on it/en Wikivoyage

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

@Andyrom75 not sure when I will have time to deploy the above change (I'm not sure if I'll get to it this week), so feel free to find somebody to deploy it without me as I don't want to be a blocker here.

Change 752751 merged by jenkins-bot:

[operations/mediawiki-config@master] Enable CirrusSearch on it/en Wikivoyage

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