Page MenuHomePhabricator

Lazy loaded images experiment breaks math images
Closed, ResolvedPublic3 Story Points


As reported on the helpdesk, the lazy loaded images experiment also seems to break the rendering of Math images.



Related Gerrit Patches:
mediawiki/extensions/MobileFrontend : wmf/1.28.0-wmf.17Ensure lazy image placeholders without height can be loaded
mediawiki/extensions/MobileFrontend : masterEnsure lazy image placeholders without height can be loaded
mediawiki/extensions/MobileFrontend : masterCopy style attribute to lazy placeholder

Event Timeline

TheDJ created this task.Aug 24 2016, 7:56 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 24 2016, 7:56 AM

In FF it looks good see screenshot but chrome does not display it. The final generated HTML code looks like that:

<p>Planck's law states that<sup id="cite_ref-Rybicki_1979_22_30-0" class="reference"><a href="#cite_note-Rybicki_1979_22-30">[30]</a></sup></p>
<dl><dd><span><span class="mwe-math-mathml-inline mwe-math-mathml-a11y" style="display: none;"><math xmlns="">
    <mrow class="MJX-TeXAtom-ORD">
      <mstyle displaystyle="true" scriptlevel="0">
        <mo stretchy="false">(</mo>
        <mo stretchy="false">)</mo>
        <mrow class="MJX-TeXAtom-ORD">
                <mrow class="MJX-TeXAtom-ORD">
              <mrow class="MJX-TeXAtom-ORD">
        <mrow class="MJX-TeXAtom-ORD">
                <mrow class="MJX-TeXAtom-ORD">
    <annotation encoding="application/x-tex">{\displaystyle I(\nu ,T)={\frac {2h\nu ^{3}}{c^{2}}}{\frac {1}{e^{\frac {h\nu }{kT}}-1}}}</annotation>
  </semantics></math></span><noscript><img src="" class="mwe-math-fallback-image-inline" aria-hidden="true" style="vertical-align: -3.505ex; width:24.412ex; height:7.343ex;" alt="I(\nu,T) =\frac{ 2 h\nu^{3}}{c^2}\frac{1}{ e^{\frac{h\nu}{kT}}-1}"></noscript><span class="lazy-image-placeholder" style="" data-src="" data-alt="I(\nu,T) =\frac{ 2 h\nu^{3}}{c^2}\frac{1}{ e^{\frac{h\nu}{kT}}-1}" data-class="mwe-math-fallback-image-inline"></span></span></dd>

The fallback image-code (that unfortunately is still required for chrome) seems to get through all rendering phases correctly and arrives as desired in the output

<img src="" class="mwe-math-fallback-image-inline" aria-hidden="true" style="vertical-align: -3.505ex; width:24.412ex; height:7.343ex;" alt="I(\nu,T) =\frac{ 2 h\nu^{3}}{c^2}\frac{1}{ e^{\frac{h\nu}{kT}}-1}">

So I don't know now what could be done from the math extension perspective.

Physikerwelt triaged this task as High priority.Aug 24 2016, 8:39 AM
Physikerwelt added a subscriber: Jdforrester-WMF.

@Physikerwelt does this use JS to load? IF so could you point me at the code?

Ignore that last comment. This is happening because the math image doesn't have a width and height attribute set.

Jdlrobson added a comment.EditedAug 24 2016, 3:28 PM

Currently generated image:

<img src=""
class="mwe-math-fallback-image-inline" aria-hidden="true" style="vertical-align: -3.505ex; width:24.412ex; height:7.343ex;" 
alt="I(\nu,T) =\frac{ 2 h\nu^{3}}{c^2}\frac{1}{ e^{\frac{h\nu}{kT}}-1}">

Image needed for lazy loading images (note width and height attributes)

<img src=""
class="mwe-math-fallback-image-inline" aria-hidden="true" style="vertical-align: -3.505ex; width:24.412ex; height:7.343ex;" 
alt="I(\nu,T) =\frac{ 2 h\nu^{3}}{c^2}\frac{1}{ e^{\frac{h\nu}{kT}}-1}">

Alternatively we can explore getting width/height out of the style attribute.

Change 306451 had a related patch set uploaded (by Jdlrobson):
Copy style attribute to lazy placeholder

Jdlrobson moved this task from Needs Analysis to Code Review on the Reading-Web-Sprint-79-Uh-oh board.

@Jdlrobson I like the idea of copying the style attribute better. However, if that does not work out, we can also add a custom workaround for the mobileFrontend extension, to the math extension. Please let me know.

Change 306451 merged by jenkins-bot:
Copy style attribute to lazy placeholder

phuedx claimed this task.Aug 25 2016, 8:32 PM

This solution causes reflows.

The style attribute is added to the placeholder element as and when we decide to load the image and not on page load.

If you throttle your browser's network connection to "sorta GPRS" mode on @bmansurov's test page, then you'll see a load of reflows as the placeholders have their dimensions set.

phuedx removed phuedx as the assignee of this task.Aug 25 2016, 9:10 PM
phuedx added a subscriber: phuedx.

@phuedx pretty sure that's always been the case. There is a display none on the container that surrounds it:

<span class="mwe-math-mathml-inline mwe-math-mathml-a11y" style="display: none;">

Worth filing a bug against Math ?

@Jdlrobson: Sorry, I wasn't being clear. On a poor quality connection, content reflows as you scroll down as the image placeholders have style attributes added to them.

We could solve this on either on the client or on the server. On the client, we'd add style attributes to the placeholders immediately, rather than as they are about to enter the viewport. On the server, we'd have to extract the width and height from the style attribute and use those values for the data-width and data-height attributes respectively.

OMG. I just tested that again. The change introduces two three new problems

  1. In introduces additional linebreaks for each formula
  2. It duplicates the Mathematical expression for clients that support MathML (screenshot)
  3. It downloads the math images from the server, even if the client supports mathml

Let's revert the new commit and let me try something new.

Change 306953 had a related patch set uploaded (by Jdlrobson):
Ensure all lazy image placeholders have a height

It duplicates the Mathematical expression for clients that support MathML (screenshot)

@Physikerwelt hi! So I downloaded a Chrome extension to support MathML and I've put the above fix on the beta cluster:

It seems to work for me but can you verify you are not seeing any further issues?
Even better if you can checkout MobileFrontend locally and tell me if the above patch works I would be most appreciative.

Jdlrobson moved this task from To Do to Code Review on the Reading-Web-Sprint-79-Uh-oh board.

That's how it looks to me. I'm testing with FF and the Native MathML extension
All what the extension does is to change the visibility of the mathml element and the class. However, someone seems to overwrite the visibility.
Moreover, it seems that elements are added after the mathml block. This causes an additional linebreak.
I have attached a second screenshot as referene how the same page looks with desktop rendering:

As one can see. With MathML rendering the \lambda_max is perfectly integrated into the text there are no additional linebreaks and the fallback images don't show up at all.

The line break issue is tracked in T143558
Right now I'm more concerned with why the image gets rendered twice. I'd need to look at that extensions implementation to understand better.

That said I think it's better to show 2 to users with that extension and 1 to users without in the meantime rather than 0. I'll have a look at the FF extension Monday.

Physikerwelt added a comment.EditedAug 27 2016, 6:24 AM

The significant line of the plugin code is
I think.
While one could argue, that only few people currently use the plugin, I would like to make this css rule default for firefox in the future again. and its parent address the loading and the line break issues. That said the Firefox extension still doesn't play nicely with it.

I don't see a nice way to fix that without modifying the firefox extension.

The extension could either:

  1. add a css rule + lazy-image-placeholder
  2. remove the .mwe-math-fallback-image-inline element from the DOM entirely

I would recommend #2 since its the most generic and least likely to break in future. This would also protect against any future changes to the Math extension which modify DOM structure (apart from if mwe-math-fallback-image-inline gets renamed of course).

The extension is only one alternative to enable MathML. Other users prefere to modify their custom CSS rules...
While the extension might ba able to modify the DOM, other approaches might not have this capability.
The function of the CSS attributes is documented here

Patch is failing to merge due to a fatal:

 Fatal error: Uncaught exception 'DBQueryError' with message ' in /mnt/jenkins-workspace/workspace/mwext-mw-selenium/src/includes/db/Database.php on line <i>1018</i>
 DBQueryError: A database error has occurred. Did you forget to run maintenance/update.php after upgrading?  See:
19:47:54       Query: SHOW MASTER STATUS
19:47:54       Function: DatabaseMysqlBase::getMasterPos
19:47:54       Error: 1227 Access denied; you need (at least one of) the SUPER,REPLICATION CLIENT privilege(s) for this operation (
19:47:54        in /mnt/jenkins-workspace/workspace/mwext-mw-selenium/src/includes/db/Database.php on line <i>1018</i></th></tr>
19:47:54       <tr><th align='left' bgcolor='#e9b96e' colspan='5'>Call Stack</th></tr>
19:47:54       <tr><th align='center' bgcolor='#eeeeec'>#</th><th align='left' bgcolor='#eeeeec'>Time</th><th align='left' bgcolor='#eeeeec'>Memory</th><th align='left' bgcolor='#eeeeec'>Function</th><th align='left' bgcolor='#eeeeec'>Location</th></tr>
19:47:54       <tr><td bgcolor='#eeeeec' align='center'>1</td><td bgcolor='#eeeeec' align='center'>0.0954</td><td bgcolor='#eeeeec' align='right'>10460088</td><td bgcolor='#eeeeec'>MediaWiki\Session\SessionManager->shutdown(  )</td><td title='/mnt/jenkins-workspace/workspace/mwext-mw-selenium/src/includes/session/SessionManager.php' bgcolor='#eeeeec'>../SessionManager.php<b>:</b>0</td></tr>
19:47:54       <tr><td bgcolor='#eeeeec' align='center'>2</td><td bgcolor='#eeeeec' align='center'>0.0955</td><td bgcolor='#eeeeec' align='right'>10460136</td><td bgcolor='#eeeeec'><a href='' target='_new'>session_write_close</a>
19:47:54       (  )</td><td title='/mnt/jenkins-workspace/workspace/mwext-mw-selenium/src/includes/session/SessionManager.php' bgcolor='#eeeeec'>../SessionManager.php<b>:</b>468</td></tr>
19:47:54       <tr><td bgcolor='#eeeeec' align='center'>3</td><td bgcolor='#eeeeec' align='center'>0.0955</td><td bgcolor='#eeeeec' align='right'>10461232</td><td bgcolor='#eeeeec'>MediaWiki\Session\PHPSessionHandler->write(  )</td><td title='/mnt/jenkins-workspace/workspace/mwext-mw-selenium/src/includes/session/SessionManager.php' bgcolor='#eeeeec'>../SessionManager.php<b>:</b>468</td></tr>
19:47:54       <tr><td bgcolor='#eeeeec' align='center'>4</td><td bgcolor='#eeeeec' align='center'>0.0955</td><td bgcolor='#eeeeec' align='right'>10461280</td><td bgcolor='#eeeeec'>MediaWiki\Session\SessionManager->getSessionById(  )</td><td title='/mnt/jenkins-workspace/workspace/mwext-mw-selenium/src/includes/session/PHPSessionHandler.php' bgcolor='#eeeeec'>../PHPSessionHandler.php<b>:</b>237</td></tr>
19:47:54       <tr><td bgcolor='#eeeeec' align='center'>5</td><td bgcolor='#eeeeec' align='center'>0.0955</td><td bgcolor='#eeeeec' align='right'>10462368</td><td bgcolor='#eeeeec'>MediaWiki\Session\SessionManager->getSessionFromInfo(  )</td><td title='/mnt/jenkins-workspace/workspace/mwext-mw-selenium/src/includes/session/SessionManager.php' bgcolor='#eeeeec'>../SessionManager.php<b>:</b>212</td></tr>
19:47:54       <tr><td bgcolor='#eeeeec' align='center'>6</td><td bgcolor='#eeeeec' align='center'>0.0956</td><td bgcolor='#eeeeec' align='right'>10463216</td><td bgcolor='#eeeeec'>ScopedCallback::consume(  )</td><td title='/mnt/jenkins-workspace/workspace/mwext-mw-selenium/src/includes/session/SessionManager.php' bgcolor='#eeeeec'>../SessionManager.php<b>:</b>883</td></tr>
19:47:54       <tr><td bgcolor='#eeeeec' align='center'>7</td><td bgcolor='#eeeeec' align='center'>0.0956</td><td bgcolor='#eeeeec' align='right'>10463264</td><td bgcolor='#eeeeec'>ScopedCallback->__destruct(  )</td><td title='/mnt/jenkins-workspace/workspace/mwext-mw-selenium/src/includes/session/SessionManager.php' bgcolor='#eeeeec'>../SessionManager.php<b>:</b>54</td></tr>
19:47:54       <tr><td bgcolor='#eeeeec' align='center'>8</td><td bgcolor='#eeeeec' align='center'>0.0956</td><td bgcolor='#eeeeec' align='right'>10463312</td><td bgcolor='#eeeeec'><a href='' target='_new'>call_user_func_array</a>
19:47:54       (  )</td><td title='/mnt/jenkins-workspace/workspace/mwext-mw-selenium/src/includes/libs/ScopedCallback.php' bgcolor='#eeeeec'>../ScopedCallback.php<b>:</b>74</td></tr>
19:47:54       <tr><td bgcolor='#eeeeec' align='center'>9</td><td bgcolor='#eeeeec' align='center'>0.0956</td><td bgcolor='#eeeeec' align='right'>10463544</td><td bgcolor='#eeeeec'>MediaWiki\Session\SessionBackend->MediaWiki\Session\{closure}(  )</td><td title='/mnt/jenkins-workspace/workspace/mwext-mw-selenium/src/includes/libs/ScopedCallback.php' bgcolor='#eeeeec'>../ScopedCallback.php<b>:</b>74</td></tr>
19:47:54       <tr><td bgcolor='#eeeeec' align='center'>10</td><td bgcolor='#eeeeec' align='center'>0.0956</td><td bgcolor='#eeeeec' align='right'>10463544</td><td bgcolor='#eeeeec'>MediaWiki\Session\SessionBackend->save(  )</td><td title='/mnt/jenkins-workspace/workspace/mwext-mw-selenium/src/includes/session/SessionBackend.php' bgcolor='#eeeeec'>../SessionBackend.php<b>:</b>596</td></tr>
19:47:54       <tr><td bgcolor='#eeeeec' align='center'>11</td><td bgcolor='#eeeeec' align='center'>0.0961</td><td bgcolor='#eeeeec' align='right'>10472800</td><td bgcolor='#eeeeec'>CachedBagOStuff->set(  )</td><td title='/mnt/jenkins-workspace/workspace/mwext-mw-selenium/src/includes/session/SessionBackend.php' bgcolor='#eeeeec'>../SessionBackend.php<b>:</b>735</td></tr>
19:47:54       <tr><td bgcolor='#eeeeec' align='center'>12</td><td bgcolor='#eeeeec' align='center'>0.0961</td><td bgcolor='#eeeeec' align='right'>10470776</td><td bgcolor='#eeeeec'>SqlBagOStuff->set(  )</td><td title='/mnt/jenkins-workspace/workspace/mwext-mw-selenium/src/includes/libs/objectcache/CachedBagOStuff.php' bgcolor='#eeeeec'>../CachedBagOStuff.php<b>:</b>63</td></tr>
19:47:54       <tr><td bgcolor='#eeeeec' align='center'>13</td><td bgcolor='#eeeeec' align='center'>0.0972</td><td bgcolor='#eeeeec' align='right'>10471176</td><td bgcolor='#eeeeec'>SqlBagOStuff->waitForSlaves(  )</td><td title='/mnt/jenkins-workspace/workspace/mwext-mw-selenium/src/includes/objectcache/SqlBagOStuff.php' bgcolor='#eeeeec'>../SqlBagOStuff.php<b>:</b>380</td></tr>
19:47:54       <tr><td bgcolor='#eeeeec' align='center'>14</td><td bgcolor='#eeeeec' align='center'>0.0972</td><td bgcolor='#eeeeec' align='right'>10471176</td><td bgcolor='#eeeeec'>LoadBalancer->getMasterPos(  )</td><td title='/mnt/jenkins-workspace/workspace/mwext-mw-selenium/src/includes/objectcache/SqlBagOStuff.php' bgcolor='#eeeeec'>../SqlBagOStuff.php<b>:</b>806</td></tr>
19:47:54       <tr><td bgcolor='#eeeeec' align='center'>15</td><td bgcolor='#eeeeec' align='center'>0.0972</td><td bgcolor='#eeeeec' align='right'>10471224</td><td bgcolor='#eeeeec'>DatabaseMysqlBase->getMasterPos(  )</td><td title='/mnt/jenkins-workspace/workspace/mwext-mw-selenium/src/includes/db/loadbalancer/LoadBalancer.php' bgcolor='#eeeeec'>../LoadBalancer.php<b>:</b>1000</td></tr>
19:47:54       <tr><td bgcolor='#eeeeec' align='center'>16</td><td bgcolor='#eeeeec' align='center'>0.0972</td><td bgcolor='#eeeeec' align='right'>10471320</td><td bgcolor='#eeeeec'>DatabaseBase->query(  )</td><td title='/mnt/jenkins-workspace/workspace/mwext-mw-selenium/src/includes/db/DatabaseMysqlBase.php' bgcolor='#eeeeec'>../DatabaseMysqlBase.php<b>:</b>849</td></tr>
19:47:54       <tr><td bgcolor='#eeeeec' align='center'>17</td><td bgcolor='#eeeeec' align='center'>0.0974</td><td bgcolor='#eeeeec' align='right'>10471632</td><td bgcolor='#eeeeec'>DatabaseBase->reportQueryError(  )</td><td title='/mnt/jenkins-workspace/workspace/mwext-mw-selenium/src/includes/db/Database.php' bgcolor='#eeeeec'>../Database.php<b>:</b>923</td></tr>

Jenkins is no longer complaining.

Change 306953 merged by jenkins-bot:
Ensure lazy image placeholders without height can be loaded

This can now be tested on - formula should show and be inline where necessary.

Note if you have the Firefox plugin images will show twice. Given most users are on mobile devices which cannot run extensions I suggest we tackle this in a separate issue. As I've pointed out I don't think this necessarily needs to be fixed in MobileFrontend.

ovasileva closed this task as Resolved.Aug 31 2016, 7:46 PM

the math is back!

Change 307971 had a related patch set uploaded (by Jdlrobson):
Ensure lazy image placeholders without height can be loaded

Change 307971 merged by Dereckson:
Ensure lazy image placeholders without height can be loaded

Mentioned in SAL [2016-09-02T00:02:31Z] <dereckson@tin> Synchronized php-1.28.0-wmf.17/extensions/MobileFrontend/resources/mobile.startup/Skin.js: Ensure lazy image placeholders without height can be loaded (T143768) (duration: 00m 46s)