64831 - Need UI for animation control

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect, P4)

Description

[Blake Ross]

See bug 17686.

Comment 1

[Matthew T (active 1999-2002)]

| Category:             Multimedia ::::::::::::::::::::::::::::::::: |
| +-------------------+                                              |
| |=General===========| [/] Automatically load ima_ges and plugins   |
| |=Display===========| [/] Allow images as _backgrounds             |
| |  Languages        | [/] Show _animations                         | <--
| |  Fonts            |     [/] Allow _looping of animations         | <--
| |  Colors & Effects | [/] Play sounds                              |
| |  Styles           |     [/] Allow loo_ping of sounds             |
| |::Multimedia:::::::| [ ] Include images and plugins in _Tab cycle |
| |  Filters          | [/] Show _frames                             |
| |  Scripts          | [/] Show _in-line frames                     |
| |  Privacy/Security | [ ] Always allow frame _resizing             |
| |                   | [ ] Always allow frame _scrolling            |
| |                   |                                              |
| +-------------------+ :::::::::::::::::::::::::::::::::::::::::::: |

Comment 2

[Jason Eager]

This is just a simple "Add prefs to XUL and JS" bug, right?

I could probably do this tonight.

Nominating for Mozilla 0.8 so this bug doesn't get left behind.

Comment 3

[Akkana Peck]

To me, something like "Animations: [/] Normal  [ ] Once  [ ] Never"
(or Continuous/Once/Never) is clearer than trying to figure out that "looping"
means "animate more than once", but maybe that's just me being geeky (I won't
argue the point if everyone else likes "looping").

Comment 4

[timeless]

I like akkana's better, although it should be (/) [silly radios] and we might 
benefit from a replacement for 'normal'.

Comment 5

[Matthew T (active 1999-2002)]

Yeah, I preferred a radio button UI until a few months ago, when I realized I'd 
never be able to come up with highly understandable text for `Normal'. The best 
I could do was `Allow animations: (*) as specified  ( ) once  ( ) never', but 
`as specified' is pretty tacky too. And what does `Allow animations ... once' 
mean? Only allow them on the first page you visit which has them? Etc.

Comment 6

[Nathan Bidwell]

Does 'Allow each animation to loop: (*) as specified  ( ) once  ( ) never' sound
more clear?

Comment 7

[timeless]

yes it does :-), ... waiting for reply from mpt.

Comment 8

[Matthew T (active 1999-2002)]

Well, I still think two checkboxes with the odd word `looping' is more usable 
than three radio buttons with the odd phrase `as specified' ... But as usual, he 
who checks in the code has the final say.

Comment 9

[Nathan Bidwell]

How about 'default' or 'forever' instead of 'as specified' since what I think
what we really mean is 'as long as it wants to'. (Now there's a clunky phrase to
use...)  Or would it confuse people to say that their images could loop
forever...?  I agree that as specified could sound like the user is supposed to
specify how many times it should loop.

Comment 10

[NilsE]

I'd vote for "as specified by image" - although not as catchy as the shorter
alternatives it's exactly what we want to say. No confusion at all. 

Comment 11

[Akkana Peck]

I like nbidwell's UI, with or without Nils' modification (depending on whether
we think we might run out of space in the window; it's better but we might not
have the space).

Comment 12

[Marcus Pallinger]

I'd prefer Default, or even Normal. To someone that doesn't understand, they may
not realise it's an image, or that it can even be specified in the image,
whereas Default or Normal would better indicate "this is what you would expect
it to do" 

Comment 13

[Greg Miller]

I think mpt's version is likely to be clearer to newbies. Might want to change
the second checkbox to "Allow animations to repeat"

Comment 14

[Johan]

How about a drop down list, containing the three options? It'd look something 
like this:
                  _____________ _
Image Animation: |_____________|*|___________
                 |Normal (Defined by Image)  |
                 |Once Only                  |
                 |Never                      |
                 -----------------------------

Not only does this minimise the window space taken by the preference, but it 
also allows for longer and clearer descriptions of each option. Also this should 
only be enabled when image loading is also enabled to make it clear to the 
user there is a link between the two.

The only problem is of course if later down the track the "Once only" option 
gets replaced by "User defined number of timer" at which time the above solution 
would no longer be workable. And it also looks different from all the other 
options on the screen (but that's not really a problem as such).

Comment 15

[Matthew T (active 1999-2002)]

> it also allows for longer and clearer descriptions of each option

Unfortunately not, because items in a popup menu should be kept very short (see 
my 2001-01-14 comment in bug 42038).

> Also this should only be enabled when image loading is also enabled to make it
> clear to the user there is a link between the two.

Ah, but there isn't. You can still load animations when automatic image loading 
is disabled -- by choosing `View Images' or `Load This Image'.

Comment 16

[Blake Ross]

So, er...now that everyone's had their two cents (and some have had more), 
anyone wanna make the final call?  tor, akk?

Comment 17

[Akkana Peck]

Not my place to make a final call, but I like Johan's wording.  I have a
preference whether it's a dropdown or radio buttons, or about the physical
layout; Blake should probably make the layout call when he implements it, based
on how much space is available in the window.

Comment 18

[Blake Ross]

*** Bug 66365 has been marked as a duplicate of this bug. ***

Comment 19

[NilsE]

I'd recommend fixing crasher bug #65016 before implementing this UI. Otherwise,
people play around with animation control and report all sorts of duplicates
that actually are #65016. (BTW, does this mean this bug depends on 65016?)

Comment 20

[Ben Bucksch (:BenB)]

> | |::Multimedia:::::::|

Why not use the Advanced|Images panel until we have a new prefs UI design?

> I'd recommend fixing crasher bug #65016 before implementing this UI.

the crash happens only for "never", right? Maybe we can just omit that option
(leaving only "once") until that bug is fixed.

Comment 21

[Stephan Niemz]

Nominating for 0.9 since 0.8 is out now

Comment 22

[Ben Bucksch (:BenB)]

Mass-change: Do not remove nominations (even if Milestone passed). Readding
mozilla0.8 nomination.

Comment 23

[HJ]

Attachment: Screenshot

Comment 24

[Akkana Peck]

I like it, except (sorry, I hate to quibble more about wording, any wording is
better than none!):
"Use animated image settings" makes me wonder "Aren't these the animated image
settings I'm changing right here?"
Clearer might be something like "Use the settings in the image" or "Use each
image's settings".

The comment about Flash seems too specific -- it would be better to make it more
general, e.g. "does not stop animations in plugins".  For instance, it won't
stop Java animations either, and I'd guess a lot more people and sites use Java
than use Flash.

Comment 25

[David Hallowell]

I also guess this controls image animation for MNG files as well as GIF (and if
it doesn't it should), so the wording should be modified to remove references to
GIF.
"This setting controls the display of animated images within Mozilla. You can
turn off animations or make the animation loop only once. This setting only
applies to images it does not affect animation in other media files"

Comment 26

[Alex Bishop]

With regards to H-J's screenshot: shouldn't the explanatory text be before the
options, or I am missing some UI convention here? Also, I think the options
should be in the reverse order, so they get less permissive the further down you
go, in line with the image blocking options.

Anyway, incorporating other people's suggestions and a few of my own, here's
what I suggest for the layout and wording:

---Image Animation---
This setting controls animated images, but does not control animation in other
media files (such as Java or Flash). By default, most images will animate
continuously (or a set number of times), but you can choose to have animations
shown only once, or not at all.

(.) Use each image's default settings
( ) Show animations only once per page visit
( ) Do not animate images

Feel free to comment on/abuse my ideas.

I tried to define the 'Normal' setting in the explanation, but I'm not exactly
sure what it means myself, so correct me if I'm wrong. Even if it is right, I
still think it's a bit kludgy. I'm a little concerned that the explanation is a
bit too long and wordy (users don't like reading!). If it is too long, how about
just: "Specify how Mozilla handles animated images (this setting does not
control animations in other media files)." The rest of the explanation could be
put in the help file.

Comment 27

[Blake Ross]

So tor, akkana, do you think this is ready for ui in .9?

Comment 28

[Akkana Peck]

If I get a review for 74169 in time to get the changes in for 0.9, then yes, I
think it will be reliable enough.  But that's just a guess -- until we actually
get that regression fixed, we won't know for sure whether it triggers any
unknown problems in libpr0n.

It does seem to work much better (with the patch in that bug) with libpr0n than
it did with the old imglib.

Comment 29

[Akkana Peck]

I checked in the animation pref backend today.  Don't know if that leaves enough
time to get the UI in or not (sorry, was backed up getting reviews).

Comment 30

[chris hofmann]

lets hold off on any new UI until 0.9.2.
let me know if this creates problems.  thanks

Comment 31

[Blake Ross]

Attachment: patch

Comment 32

[Blake Ross]

Attachment: screenshot

Comment 33

[Hixie (not reading bugmail)]

r=hixie at the code level, mpt, could you review the screenshot?

Comment 34

[Ben Goodger (use ben at mozilla dot org for email)]

How about

+--- Animated Images should loop: -------------+
| (*) ...

Comment 35

[Ben Goodger (use ben at mozilla dot org for email)]

And add a newline to the end of the file ;) 

Comment 36

[Ben Goodger (use ben at mozilla dot org for email)]

sr=[e-mail] with those changes ('Image Looping' just seemed redundant,
given only one set of options with a single statement). 

Comment 37

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to the trunk.
(on behalf of drivers)

Comment 38

[doctor__j]

Question: If I want to let the GIF loop as many times as it was
specified/designed, which radio button should I choose?

Comment 39

[Blake Ross]

Good point....hrm....

"As many times as the image specifies"?  I guess that's pretty wordy.  Other 
suggestions?

Comment 40

[Warner Young]

Loop as many times as specified should just be "Default," I would think.

Comment 41

[doctor__j]

I raise this question because I found out from Bonsai that some files
(pref-images.xul, etc.) have been checked in.  I am afraid there's no "default"
button for me to choose...  The 3 radio buttons are only "LoopForever",
"LoopOnce" and "LoopNever".

Comment 42

[robbe]

Wouldn't people wonder: "what is the Default, then??"

So ... what about "as specified by animation"?

Comment 43

[Blake Ross]

UI for this has been checked in, with the wording "As many times as the image 
specifies".  Yes, the wording needs work (for example, the sentence 
reads "Animated images should loop as many times as the image specifies").  I 
just wanted to get it in since it's just barely missed every milestone 
since .8.1, and .9.2 is coming very shortly.  I'll leave this open to discuss 
better wording, and then we'll fix it up.  I'd really rather stick with the 
radiobutton UI.

Comment 44

[Alex Bishop]

Here's my latest and greatest thoughts on the wording:

-----Image Animations---------------

(.) Show and repeat image animations
( ) Show image animations once only
( ) Do not show image animations

I used "repeat" rather than "loop" because I think it will be understood better
by most people. The only problem is that by default some images animate only
once in which case the text for the first button isn't strictly accurate.

Comment 45

[Alex Bishop]

Actually, I've changed my mind. The title of the setting should be "Animated
Images" (not "Image Animations").

Comment 46

[Blake Ross]

If people are still interested in pursuing this, please move the discussion to 
the ui newsgroup, and then file a bug on the consensus.

Comment 47

[Matthew T (active 1999-2002)]

The groupbox used in this UI falls off the edge of the prefs panel in Mac Classic.

Comment 48

[Blake Ross]

Sheesh. I can't believe this dialog doesn't have vertical and horizontal 
scrollbars.  All the other oversized dialogs I see in applications do...

Comment 49

[Matthew T (active 1999-2002)]

Hmmmm, I think a sledgehammer is needed to solve this problem.