191166 - animated gifs animate in Composer when they shouldn't

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Kathleen :Brade]

animated gifs currently can animate in Composer when they shouldn't (except in
the image preview dialog?).  In editorshell, we made an explicit call so gifs
wouldn't animate.  That call is missing in nsEditingSession.cpp

Comment 1

[Roland Mainz]

smontagu:
Remember your "trick" how to did turn-off GIF animation for PrintPreview ? WOuld
the same solution work for this bug, too ?

Comment 2

[Simon Montagu :smontagu]

That "trick" was dependent on print and print preview having their own
subclasses of nsPresContext in which we could override SetImageAnimationMode()
and GetImageAnimationMode(). As far as I know Composer uses the base
nsPresContext so that wouldn't work.

Comment 3

[Roland Mainz]

... and we can't subclass and/or hack the plain version and add a flag which
says "don't animate", right (AFAIK we still have the bug to disable image
animations on demand(=prefs) open (=not fixed since four years)) ?

Comment 4

[Kathleen :Brade]

the fix for editor is to make this call:
  presContext->SetImageAnimationMode(imgIContainer::kDontAnimMode);

Comment 5

[Simon Montagu :smontagu]

Re comment 4: that was the original fix for print preview (in bug 2586),
but there were cases where the image animation mode was apparently
getting reset (see bug 182259 and bug 133808), so proceed with caution :-)

Re comment 3: the pref to stop image animation worksforme.

Comment 6

[Kathleen :Brade]

Attachment: patch that disables animated gifs as editorshell used to (but with more error checking)

Comment 7

[Kathleen :Brade]

Comment on attachment 113737 [details] [diff] [review]
patch that disables animated gifs as editorshell used to (but with more error checking)

btw, the printf cleanup was just to remove warnings

Comment 8

[timeless]

Comment on attachment 113737 [details] [diff] [review]
patch that disables animated gifs as editorshell used to (but with more error checking)

@@ -893,6 +895,20 @@
>+  // Disable animation of GIFs in this document:
if the api is generic and applies to MNG and ICO then your comment shouldn't
explicitly reference GIF :)

Comment 9

[kinmoz]

Comment on attachment 113737 [details] [diff] [review]
patch that disables animated gifs as editorshell used to (but with more error checking)

The changes look ok to me, but the fact that it is in EndDocumentLoad() means
that content/iframes that are already loaded and have their edit bit flipped
will still have animations right? Do we need to put this in
SetupEditorOnWindow?

Also should this be something that is configureable by the embeddor?

Comment 10

[Kathleen :Brade]

Attachment: move changes to SetupEditorOnWindow as suggested by Kin

Comment 11

[Simon Fraser [no longer active]]

I think we have to turn off meta-refresh, JS, and maybe animaged GIFs for every
subframe in composer, whether it has been made editable or not. The reason is
that JS or a meta-refresh running in a non-editable subframe might cause a URL
to load in "top", blowing away all the other frames.

Comment 12

[Kathleen :Brade]

Attachment: updated patch which doesn't move refreshuri

Comment 13

[Simon Fraser [no longer active]]

Comment on attachment 114235 [details] [diff] [review]
updated patch which doesn't move refreshuri

Please diff with enough context to show the function names, if possible.

Comment 14

[kinmoz]

Comment on attachment 114235 [details] [diff] [review]
updated patch which doesn't move refreshuri

sr=[e-mail]

Comment 15

[Kathleen :Brade]

this is fixed