Closed Bug 1692736 Opened 3 years ago Closed 3 years ago

animated gif is stuck if canvas is also rendered

Categories

(Core :: Graphics: WebRender, defect, P1)

Firefox 87
defect

Tracking

()

VERIFIED FIXED
87 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox85 --- unaffected
firefox86 + verified
firefox87 --- verified

People

(Reporter: tristan.fraipont, Assigned: longsonr)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

Attached file test-case.html

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.16; rv:85.0) Gecko/20100101 Firefox/85.0

Steps to reproduce:

https://jazzy-placid-sunspot.glitch.me/

Display an animated gif on the page and draw on a canvas every frame at the same time.

Actual results:

The gif animation stutters, it only animates sometimes when the full page is rerendered (e.g after a scroll or a window resize).

Expected results:

The gif should still animate smoothly.

Running mozregression points to this pushlog, hence asking to aosmond@mozilla.com if they can have a look, since it seems bug1691065 is gonna land soon.

Flags: needinfo?(aosmond)

Confirmed the regression range in the first comment.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Regressed by: 1691065
Has Regression Range: --- → yes

Set release status flags based on info from the regressing bug 1691065

Assignee: nobody → longsonr
Status: NEW → ASSIGNED

Looks like this could have been caught if he had bug 1602142 ?

Pushed by longsonr@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f8c9e66702b5
pass namespace id when writing/reading TransactionData r=aosmond

Comment on attachment 9203355 [details]
Bug 1692736 - pass namespace id when writing/reading TransactionData

Beta/Release Uplift Approval Request

  • User impact if declined: animated gifs fail to animate
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: The testcase in this bug should animate . Hopefully this also fixes bug 1692916 which has its own steps to reproduce.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Just passes a value to the child process, The bigger change was the bug that caused this issue i.e. bug 1691065 and that's already on beta so your choice would seem to be take this or backout that one. This is a pretty small change and backing out the bigger fix would re-introduce the issues that it fixes so on the whole I'd say taking this is the least risk option.
  • String changes made/needed: none
Attachment #9203355 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Was about to request beta uplift, thanks very much for that and the patch fixing my mistake :).

Flags: needinfo?(aosmond)
Severity: -- → S3
Priority: -- → P1
Attachment #9203355 - Flags: approval-mozilla-beta? → approval-mozilla-release?
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 87 Branch
QA Whiteboard: [qa-triaged]

Reproduced the issue on Nightly 87.0a1 (20210216094005)
Verified the fix on Nightly 87.0a1 (20210216215129)

This would break animated images mostly, some font cases. I think this is pretty important for an RC uplift, so setting to S2. The workaround is disable WebRender for a user which is pretty drastic.

Severity: S3 → S2

(In reply to Robert Longson [:longsonr] from comment #6)

Comment on attachment 9203355 [details]
Bug 1692736 - pass namespace id when writing/reading TransactionData

Beta/Release Uplift Approval Request

  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Just passes a value to the child process, The bigger change was the bug that caused this issue i.e. bug 1691065 and that's already on beta so your choice would seem to be take this or backout that one. This is a pretty small change and backing out the bigger fix would re-introduce the issues that it fixes so on the whole I'd say taking this is the least risk option.

I would concur we must either back out bug 1691065 (which helped a lot with the crash rate otherwise, notwithstanding this oversight) or take the patch in this bug. Keeping bug 1691065 and not taking bug 1692736 is not desirable for the release.

Comment on attachment 9203355 [details]
Bug 1692736 - pass namespace id when writing/reading TransactionData

I agree that this has the potential to annoy a lot of users and the fix is small and was verified by QA, I consider this as a driver for an RC2, thanks for the feedback!

Attachment #9203355 - Flags: approval-mozilla-release? → approval-mozilla-release+

Verified the fix on Firefox 86.0 (20210217195321)

QA Whiteboard: [qa-triaged]
Flags: qe-verify+
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.