1366875 - SVG image element flickers when updating the value of the xlink:href attribute

Status: VERIFIED FIXED

(Core :: SVG, defect)

Description

[Nick]

Attachment: HTML file with javascript that demonstrates the bug.

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.96 Safari/537.36

Steps to reproduce:

1) Create an SVG element with an <image> element with a xlink:href attribute. Set the value of said attribute to the URL of an image. Example:

<svg viewBox="0 0 1920 1080">
  <image xlink:href="https://placehold.it/1920x1080?1" />
</svg>

2) Create an Image object with javascript and attach an onload callback function. Example:

var image = new Image();
image.onload = imgLoaded;

3) Set the src of the Image object to something other than the current value of the <image> element's xhref:link attribute. Example:

image.src = 'https://placehold.it/1920x1080?2';

4) In the onload callback function, set the value of the xhref:link attribute in the <image> element to the src of the image object. Example:

function imgLoaded() {
  document.getElementsByTagName('image')[0].setAttribute('xlink:href', 'https://placehold.it/1920x1080?2');
}

The bug can be reproduced in Firefox 53 and Nightly with build ID 20170522030207. It can not be reproduced in Firefox 52.


Actual results:

The image flickers (i.e. there's a delay between the first image and the second image causing a blink effect).


Expected results:

The second image should be shown without any flicker.

Comment 1

[Nick]

I mistakenly referred to the image element's attribute as xhref:link in a couple of places. It should be xlink:href and nothing else.

Comment 2

[Robert Longson [:longsonr]]

Given that this is a regression from 52 would you mind using mozregression(http://mozilla.github.io/mozregression/) to find a regression range?

Comment 3

[Loic]

(In reply to Robert Longson from comment #2)
> Given that this is a regression from 52 would you mind using
> mozregression(http://mozilla.github.io/mozregression/) to find a regression
> range?

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f608a5a5d8e2472396a0f4cd04778730866b8ae0&tochange=a511e97862314dc1b90c531205489b972f5d50bd

Timothy Nikkel — Bug 1341881. Don't do sync decode for SVG <image> elements images. r=aosmond

Comment 4

[Timothy Nikkel (:tnikkel)]

Attachment: patch

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/689dc2a3119a
Apply the same sync decoding heuristic to SVG <image> as we do to HTML <img>. r=mats

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/689dc2a3119a

Comment 7

[Liz Henry (:lizzard) (relman/hg->git project)]

Wow, that was fast!  theg72, want to verify this fix in tomorrow's Nightly build?
We should probably also uplift this to beta 54.

Comment 8

[Nick]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #7)
> Wow, that was fast!  theg72, want to verify this fix in tomorrow's Nightly
> build?
> We should probably also uplift this to beta 54.

Awesome, that was really fast indeed! I'd be happy to verify it in tomorrow's nightly. Thanks!

Comment 9

[Nathan Froyd [:froydnj]]

Want to request uplift of this ahead of verification? :)

Comment 10

[Nick]

(In reply to theg72 from comment #8)
> (In reply to Liz Henry (:lizzard) (needinfo? me) from comment #7)
> > Wow, that was fast!  theg72, want to verify this fix in tomorrow's Nightly
> > build?
> > We should probably also uplift this to beta 54.
> 
> Awesome, that was really fast indeed! I'd be happy to verify it in
> tomorrow's nightly. Thanks!

I have tested on the latest Nightly build, and the image still blinks once (the first time the image updates), but I can live with that. It's definitely much better than before when it kept flickering continuously. Thanks to everyone involved for resolving this so quickly, much appreciated!

Comment 11

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8870272 [details] [diff] [review]
patch

Approval Request Comment
[Feature/Bug causing the regression]: bug 1341881
[User impact if declined]: flickr when setting src in svg <image>
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: this applies the same sync decoding logic from html <img> that we've had for years to svg <image>
[String changes made/needed]: none

Comment 12

[Gerry Chang [:gchang]]

Comment on attachment 8870272 [details] [diff] [review]
patch

Improve the SVG image flickering issue and was verified. Beta54+. Should be in 54 beta 11.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

This needs a rebased patch for Beta.

Comment 14

[Timothy Nikkel (:tnikkel)]

Attachment: patch for beta

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/9f43724221c5

Comment 16

[Bogdan Maris, Desktop Test Engineering]

Hello Nick, I could be missing something but I can't seem to reproduce the issue on the same builds you reproduced (Fx 53.0 and Nightly with buildID: 20170522030207). I just loaded the .html you attached and nothing happens, also modified it based on comment 1.

I need to reproduce so I can verify on 54.0 RC2 (https://archive.mozilla.org/pub/firefox/candidates/54.0-candidates/build3/).

Comment 17

[Nick]

(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #16)
> Hello Nick, I could be missing something but I can't seem to reproduce the
> issue on the same builds you reproduced (Fx 53.0 and Nightly with buildID:
> 20170522030207). I just loaded the .html you attached and nothing happens,
> also modified it based on comment 1.
> 
> I need to reproduce so I can verify on 54.0 RC2
> (https://archive.mozilla.org/pub/firefox/candidates/54.0-candidates/build3/).

Hi Bogdan,

Can you please try this jsfiddle that I put together: https://jsfiddle.net/2dce2fj8/1/. I can reproduce the issue with this fiddle in Firefox 53.0.3 on Windows 10.

Comment 18

[Bogdan Maris, Desktop Test Engineering]

(In reply to Nick from comment #17)
> (In reply to Bogdan Maris, QA [:bogdan_maris] from comment #16)
> > Hello Nick, I could be missing something but I can't seem to reproduce the
> > issue on the same builds you reproduced (Fx 53.0 and Nightly with buildID:
> > 20170522030207). I just loaded the .html you attached and nothing happens,
> > also modified it based on comment 1.
> > 
> > I need to reproduce so I can verify on 54.0 RC2
> > (https://archive.mozilla.org/pub/firefox/candidates/54.0-candidates/build3/).
> 
> Hi Bogdan,
> 
> Can you please try this jsfiddle that I put together:
> https://jsfiddle.net/2dce2fj8/1/. I can reproduce the issue with this fiddle
> in Firefox 53.0.3 on Windows 10.

Thanks for this Nick!
Unfortunately I can see the one time flicker on 53.0.3 (bad build) as well as on 54.0 RC2 (good build). I also tried with the Nightly build from 2017-05-22 but it's the same for me, only one time flicker per update, note that I used Windows 64bit and macOS 10.12.5. 

Could you try on your side with the 54 build, if it works for you then I can close this bug? link is in comment 16.

Comment 19

[Nick]

(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #18)
> (In reply to Nick from comment #17)
> > (In reply to Bogdan Maris, QA [:bogdan_maris] from comment #16)
> > > Hello Nick, I could be missing something but I can't seem to reproduce the
> > > issue on the same builds you reproduced (Fx 53.0 and Nightly with buildID:
> > > 20170522030207). I just loaded the .html you attached and nothing happens,
> > > also modified it based on comment 1.
> > > 
> > > I need to reproduce so I can verify on 54.0 RC2
> > > (https://archive.mozilla.org/pub/firefox/candidates/54.0-candidates/build3/).
> > 
> > Hi Bogdan,
> > 
> > Can you please try this jsfiddle that I put together:
> > https://jsfiddle.net/2dce2fj8/1/. I can reproduce the issue with this fiddle
> > in Firefox 53.0.3 on Windows 10.
> 
> Thanks for this Nick!
> Unfortunately I can see the one time flicker on 53.0.3 (bad build) as well
> as on 54.0 RC2 (good build). I also tried with the Nightly build from
> 2017-05-22 but it's the same for me, only one time flicker per update, note
> that I used Windows 64bit and macOS 10.12.5. 
> 
> Could you try on your side with the 54 build, if it works for you then I can
> close this bug? link is in comment 16.

Hi Bogdan, sorry for the delayed response. I have tried with 54.0 (the actual release version) on different platforms, and strangely enough the flicker still persists in the fiddle I sent you (at least on my Windows laptop), but it has stopped in the web app I'm developing for a client, which is what really matters. There's still that one time flicker, but I think that's good enough and I will consider the issue as resolved as far as I'm concerned. So please feel free to close this bug. Thank you!

Comment 20

[Bogdan Maris, Desktop Test Engineering]

(In reply to Nick from comment #19)
> (In reply to Bogdan Maris, QA [:bogdan_maris] from comment #18)
> > (In reply to Nick from comment #17)
> > > (In reply to Bogdan Maris, QA [:bogdan_maris] from comment #16)
> > > > Hello Nick, I could be missing something but I can't seem to reproduce the
> > > > issue on the same builds you reproduced (Fx 53.0 and Nightly with buildID:
> > > > 20170522030207). I just loaded the .html you attached and nothing happens,
> > > > also modified it based on comment 1.
> > > > 
> > > > I need to reproduce so I can verify on 54.0 RC2
> > > > (https://archive.mozilla.org/pub/firefox/candidates/54.0-candidates/build3/).
> > > 
> > > Hi Bogdan,
> > > 
> > > Can you please try this jsfiddle that I put together:
> > > https://jsfiddle.net/2dce2fj8/1/. I can reproduce the issue with this fiddle
> > > in Firefox 53.0.3 on Windows 10.
> > 
> > Thanks for this Nick!
> > Unfortunately I can see the one time flicker on 53.0.3 (bad build) as well
> > as on 54.0 RC2 (good build). I also tried with the Nightly build from
> > 2017-05-22 but it's the same for me, only one time flicker per update, note
> > that I used Windows 64bit and macOS 10.12.5. 
> > 
> > Could you try on your side with the 54 build, if it works for you then I can
> > close this bug? link is in comment 16.
> 
> Hi Bogdan, sorry for the delayed response. I have tried with 54.0 (the
> actual release version) on different platforms, and strangely enough the
> flicker still persists in the fiddle I sent you (at least on my Windows
> laptop), but it has stopped in the web app I'm developing for a client,
> which is what really matters. There's still that one time flicker, but I
> think that's good enough and I will consider the issue as resolved as far as
> I'm concerned. So please feel free to close this bug. Thank you!

Thanks you so much Nick! Changing the tracking flag for 54 to verified based on reporter verification.

Comment 21

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

Hi Nick,

Could you try on your side with the 55 build, if it works for you, then I can close this bug?

Thanks, 
       Timea

Comment 22

[Nick]

(In reply to Timea Zsoldos from comment #21)
> Hi Nick,
> 
> Could you try on your side with the 55 build, if it works for you, then I
> can close this bug?
> 
> Thanks, 
>        Timea

Hi Timea,

The 55 build works the same as the 54 build for me (see comment #19). As far as I'm concerned, you can go ahead and close this bug. Thanks!

Comment 23

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

Thanks you very much Nick! I am changing the tracking flag for firefox 55 to verified fixed, close this bug.