xiaotong Reporter
	Description • 25 years ago

This is open to address the accessibility issues according to W3C accessibility
guidelines. The priority 1 requirements for user interface:
Allow the user to turn on and off animated or blinking text.
Allow the user to run on and off animations and blinking images.

	shuang (gone)
	Comment 1 • 25 years ago

don't know whether it is possible to turn off animated or blink image within
browser. german, any idea?

	german
	Comment 2 • 25 years ago

I don't think this is possible, but let's check with the apps team: Don?

	don
	Comment 3 • 25 years ago

We sort of do this in 4.x anyway.

Move to M15 for later evaluation.

	chuckie
	Comment 4 • 25 years ago

I'm not sure how this will work with blinking images, but what if blinking text
stopped blinking when the user highlighted the text?  Just an idea.

	Matthew Tuck [:CodeMachine]
	Comment 5 • 25 years ago

Animated images is covered over at bug #15145.  Don't think the others are
covered anywhere.

Chuckie, I think the idea with blinking text is that it never blinks due to
medical conditions like epilepsy.

	leger
	Comment 6 • 25 years ago

Moving all UE/UI bugs to new component: User Interface: Design Feedback
UE/UI component will be deleted.

	don
	Comment 7 • 25 years ago

Move to M16 for now ...

	timeless
	Comment 8 • 24 years ago

Relnote: ~If you have epilepsy, please consider using a mozilla derivative that
does not include a gui~

Does this still require architectural changes?

Hixie, assuming !important worked, could it prevent blink?

	Hixie (not reading bugmail)
	Comment 9 • 24 years ago

Only by preventing all underlining, overlining and strikethrough, and it would
not be able to turn off animated images without turning _all_ of them off.

	David Baron :dbaron:
	Comment 10 • 24 years ago

That's not really true -- we could have a separate user pref that disables
'blink' (and animation, but that's separate from CSS), since blink is optional.

	Hixie (not reading bugmail)
	Comment 11 • 24 years ago

I was answering the "assuming !important worked, could it prevent blink" 
question, and assuming this was referring to the user stylesheet, in which
context I stand by my statement.

However, yes, the way to fix this is indeed by using a pref and making the
code override blink and animated images based on it.

	Matthew Tuck [:CodeMachine]
	Comment 12 • 24 years ago

I don't believe blink and animation should be the same pref.  I wouldn't want
blink but would want animation.  Disabling animation is already fairly advanced
over at bug #17686.

As a temporary thing perhaps we could ensure blink is at a period that is known
not to effect epileptics.  I haven't heard of it being a problem in previous
browsers, I would suspect general video is a much bigger problem since it could
go a lot faster.

	Stuart Ballard
	Comment 13 • 24 years ago

From II Mozilla, Caput XXVII (http://www.gate.net/~shipbrk/mozilla.html):

22     And the Lizard named several of the locusts, and regarding one the Lizard
said, The <blink> tag worketh not. Whereupon the users hearing this were sore
amazed, and said they one unto another, Verily, that is no bug, but a feature to
be highly praised while it lasteth.

(seriously though... I'm surprised that the blink tag is even supported...)

	Viswanath Ramachandran
	Comment 14 • 24 years ago

Since Don has left, Vishy is taking his bugs in bulk, pending reassignment.
thanks,
	Vishy

	Zach Lipton [:zach]
	Comment 15 • 24 years ago

Chaning the qa contact on these bugs to me. MPT will be moving to the 
owner of this component shortly. I would like to thank him for all his hard 
work as he moves roles in mozilla.org...Yada, Yada, Yada...

	Skewer
	Comment 16 • 24 years ago

This bug gets my vote, and it needs to be nsCatFood too. Nobody likes blinking 
text! It needs to be optional. I want a way to disable blinking text, both in 
the BLINK element and CSS1 text-decoration: blink;. This needs to be accessible 
through the UI and fairly simple to do (clear one checkbox, for example). This 
is a serious enough user satisfaction issue to cause people to switch to a 
different browser. In fact, when I ask people why they don't use Netscape, 
BLINK is usually at the top of their list.

	Hixie (not reading bugmail)
	Comment 17 • 24 years ago

The proportion of pages with blinking text is minute. I can't recall the last
time I saw blinking text, in fact, and I use Mozilla on a daily basis.

	Skewer
	Comment 18 • 24 years ago

That's no reason not to make it optional.

	Hixie (not reading bugmail)
	Comment 19 • 24 years ago

I didn't say it was; I was merely stating surprise at your statement that "in 
fact, when [you] ask people why they don't use Netscape, BLINK is usually at 
the top of their list".

	Zach Lipton [:zach]
	Comment 20 • 24 years ago

Skewer:

>That's no reason not to make it optional.

I must disagree with this statement. Contributors are only able to do so 
much in such an amount of time. Every feature added and the time 
debuging those new features takes time away from fixing the really 
important bugs that we have to deal with. This alone is reason enough to 
not make it optional.

	Skewer
	Comment 21 • 24 years ago

Then why even have BLINK in the first place? The time people spent writing the
code to support BLINK could have been spent writing code compatible with W3C
specifications instead of propriatery (and extremely annoying) text effects...

	Hixie (not reading bugmail)
	Comment 22 • 24 years ago

To make <blink> work takes one line of CSS. That's it. Nothing else. It probably
took someone around 20 seconds. That's hardly enough time to spend "writing code
compatible with W3C specifications instead of propriatery [sic] (and extremely
annoying) text effects".

Anyway, let's focus on what the bug asks for, namely, a way to disable blinking
and animations (independently). Does anyone have the time to implement this?

	Matthew Tuck [:CodeMachine]
	Comment 23 • 24 years ago

It was my understanding that this is a part of the accessibility effort.

	Matthew T (active 1999-2002)
	Comment 24 • 24 years ago

I don't think this needs any extra UI, beyond that provided for other 
animations. If you want animations but not blinking text, you can edit your 
html.css. But I think most people who don't like flashy moving things will want 
to toggle both at once.

| Category:             Multimedia :::::::::: [  Normal settings :^] |
| +-------------------+                                              |
| |=Navigator=========| [/] Automatically load ima_ges and plugins   |
| |=Display===========|     [/] Allow images as _backgrounds         |
| |  Languages        | [/] Show _animations                         |
| |  Accessibility    |     [/] Allow _looping of animations         |
| |  Fonts            | [/] Play _sounds                             |
| |  Colors & Effects |     [/] Allow loo_ping of sounds             |
| |::Multimedia:::::::|                                              |

As a preliminary implementation, turning off `Allow looping of animations' 
would disable blinking text completely. As a further enhancement, turning off 
`Show animations' would make blinking text only blink three times before 
stopping.

	Matthew T (active 1999-2002)
	Comment 25 • 24 years ago

Errr ... Make that, `Having "Show animations" turned on, but "Allow looping of 
animations" turned off, would make blinking text blink only three times before 
stopping'. Sorry for the confusion.

	Skewer
	Comment 26 • 24 years ago

I think blinking text needs its own option. Animated images are usually done
tastefully on the web, and the functionality of some websites depends on the
viewer's ability to view animated graphics. Blinking text is totally different.
It's like pestilence in my browser. It serves no purpose except to annoy.

	Skewer
	Comment 27 • 23 years ago

OK, here's what we need. Right now, if we just want to disable BLINK tags, we
just have to change the entry in html.css to this:

blink {
/*  text-decoration: blink;*/
  font-weight: bold;
}

However, anyone who uses the CSS text-decoration: blink; will still be able to
create this irritating effect. Adding UI to change html.css would be only a
partial solution, we need UI to disable text-decoration: blink; (and therefore,
a corresponding entry in prefs.js).

	Skewer
	Comment 29 • 23 years ago

Hey, looks like someone's ahead of us. Blinking text appears to be disabled in
2001061504 Win98.

	Aaron Leventhal
	Comment 30 • 23 years ago

Adding fcc508 keyword.

This is needed for FCC Section 508 compliance.
Without this option, Mozilla based products cannot be sold to US Federal
Government agencies.

	Skewer
	Comment 31 • 23 years ago

This appears to be fixed to me. Animated images can be set to loop normally,
once, or never in the Privacy and Security > Images pref. Blinking text doesn't
blink anymore. Embedded sounds are handled by plugins.

	Aaron Leventhal
	Comment 32 • 23 years ago

On one hand, I'm glad we have the animation pref now.

However, how is a user supposed to find animation prefs under "Privacy and
Security". What do they have to do with that?

IMO, no one looking for animation or image prefs would look there - they would
look under "Appearance" prefs or something.

	timeless
	Comment 33 • 23 years ago

aaron, that's covered in another bug

	Hixie (not reading bugmail)
	Comment 34 • 23 years ago

Someone should open a bug for disabling 'blink' (the fact that it does not work
right now is a bug, and it will work eventually).

	Skewer
	Comment 35 • 23 years ago

If bug 89065 is fixed, this bug will be reopened with this summary. There's not
much chance we'll lose the animation pref now, and this bug already has the
votes, testcase, keywords, and comments.

	Skewer
	Comment 36 • 23 years ago

And this is a 4xp if you consider the

user_pref("browser.blink_allowed", false);

workaround which worked in older Netscape 4 builds (but for some reason doesn't
work in the latest NS4 build).

	Christopher Hoess (gone)
	Comment 37 • 23 years ago

This bug isn't "fixed", there's no user pref.  Reopening and adding dependency on 
bug 89065.

	Chase Tingley Assignee
	Comment 38 • 23 years ago

Taking this; I'm an inch away from fixing bug 89065, so I owe it to the world to
fix this bug as well.

	Chase Tingley Assignee
	Comment 40 • 23 years ago

This adds a 4.x-esque "browser.blink_allowed" pref, which defaults to true.  I
added it to the prefs to which nsPresContext listens so that it would be easily
accessible when building the reflow state...

The patch doesn't do anything about a UI, including tying this to animation
looping.  mpt told me on #mozilla that he still believes in this, but given the
current state of the animation loop UI, I don't understand how that's intuitive
at all:

+ Animated images should loop ---------------+
| (*) As many times as the image specifies   |
| ( ) Once                                   |
| ( ) Never                                  |
+--------------------------------------------+

At any rate, there are two separate things going on.  I'd like to try to get
reviews on a pref-only fix now to limit the blink-inflicted suffering on the
trunk, while discussion continues about the UI issue.

cc'ing ftang and attinasi, who seem (from the cvs logs) to know this code well.

	Skewer
	Comment 41 • 23 years ago

I think the best existing pref panel to put this on is Appearance/Fonts, as a
checkbox:

[*] Allow documents to use other fonts
[ ] Allow documents to display blinking text

I can't say that I agree with the decision to make blinking text enabled by
default, but I suppose if it wasn't, nobody would ever enable it.

By the way, it would be a good idea to tweak the blink rate so blinking text is
* easier to read
* has less of a headache-inducing strobe effect
For example, it could be on for three seconds, then off for half a second. But
that is a distant second choice to adding a pref.

	Marc Attinasi
	Comment 42 • 23 years ago

I think you are better off defaulting the value of the pref in the code and NOT
adding the pref to the all.js file. Simply remove the pref from all.js, and in
the presShell code initialize boolPref to PR_TRUE before calling GetBoolPref,
and then you can ignore the result of that call too. We should not have to add
default values to the pref file.

	Chase Tingley Assignee
	Comment 43 • 23 years ago

Comment on attachment 53052 [details] [diff] [review]
patch - support for "browser.blink_allowed"

Good idea.

	Marc Attinasi
	Comment 45 • 23 years ago

Comment on attachment 53197 [details] [diff] [review]
v2 -- assume a default value instead of adding to all.js

sr=attinas

	Skewer
	Comment 46 • 23 years ago

Since this is important for a lot of reasons (accessibility to users with
epilepsy, FCC requirements), I think we need to get a front-end built to make
the choice available to an end-user. Organizations which would rather not have
blinking text won't go to the trouble to edit JS config files to remove it - not
when IE doesn't use it at all. This may be something that needs to be moved to a
new bug, but keep it in mind.

	David Baron :dbaron:
	Comment 47 • 23 years ago

I seem to recall that it was preferred that there always be a default in all.js,
although I could be wrong.

Why does the pres context need to know about this pref?  Can't it all be handled
from a single static prefchange callback in the reflow state and two static
variables (an initialized boolean, and a blink pref boolean)?  Wouldn't that be
preferable? (It avoids the virtual function call and avoids increasing the size
of the pres context.)  But maybe I'm being silly...  Do we handle most other
similar prefs in the pres context?

	Marc Attinasi
	Comment 48 • 23 years ago

I think it was put in the PresContext because we have several similar prefs
there (page and link colors, for example). Maybe they started out there because
the presContext is so widely available to layout? I agree that it is worth
trying to move the prefs out of there - the PresContext is turning into a
dumping grounds. I think that should be another bug though, one which I will
open if you agree.

	Chase Tingley Assignee
	Comment 49 • 23 years ago

I'm open to either solution -- there's no reason why a single static callback
wouldn't suffice; I was following the example of the link color, etc, prefs
already in the pres context (as you suggested).  I can clean this up now to save
some work later and avoid bloat.

Also, the more I ask around the more it sounds like dbaron is right about
including defaults in all.js.

Writing this one once more would also let me clean up that bogus logic with the
NS_FAILED call, which has been bugging me (should default to true, ignore rv).

Attinasi, any objections?  

Pulling this in; I'd like to get this into 0.9.6 on humanitarian grounds.

	Marc Attinasi
	Comment 50 • 23 years ago

No objects from me. I am curious why we want default values in all.js - won;t
that just make the file larger (a minus for distribution) and slower to load (a
minus for performance)? I guess it would be nice to see all the values that have
meaning (a plus for usability).

	Akkana Peck
	Comment 51 • 23 years ago

I thought storing default values was the whole point of all.js and friends.  (As
well as being good documentation -- the only documentation -- for what the
default values are.)  If not that, then what's it for?  If we have performance
problems in reading lines from all.js, then we're in trouble anyway and ought to
rethink our prefs strategy.  But it doesn't seem to be showing up as a problem
on profiling runs.

	Chase Tingley Assignee
	Comment 52 • 23 years ago

Comment on attachment 53197 [details] [diff] [review]
v2 -- assume a default value instead of adding to all.js

bam!

	Marc Attinasi
	Comment 53 • 23 years ago

I thought all.js and friends were for setting values *other than* the defaults,
such that they apply to all profiles. 

Besides the documentation benefits, what is the motivation to put all default
values in the pref file? Even if there is not performance problem reading large
pref files, it cannot be any faster than reading small ones! And we *do* have a
performance problem in loading - it may not be due to the prefs, but anything
done at load time contributes to how ong it takes to get to your first webpage...

BUT - I don't really care too much here - if everybody thinks we should have
default values in all.js that is cool with me, I just would like to know why.
And I apologize for cluttering this bug with this tangential issue.

	Chase Tingley Assignee
	Comment 55 • 23 years ago

The default pref is back in all.js by popular demand.  The code has been moved
entirely into nsHTMLReflowState.cpp and rewritten to use a static callback and
associated state.

	David Baron :dbaron:
	Comment 56 • 23 years ago

Wouldn't it be more logical to write:

  mBlinks = (parentReflowState && parentReflowState->mBlinks);
  if (!mBlinks && BlinkIsAllowed()) {

?  After all, the pref won't change during a single reflow.  Other than that, 
r=dbaron

	rbs
	Comment 57 • 23 years ago

I wonder why there seems to be little use of the helpful
nsPresContext::GetCachedBoolPref(). It seems appropriate here.

	David Baron :dbaron:
	Comment 58 • 23 years ago

Why should we waste time registering and unregistering callbacks for each pres
context created and destroyed when we want to keep the pref at one place in
static data and can always know what it is?

	rbs
	Comment 59 • 23 years ago

If that's what you intended then the patch does the job. Forget my comments.

	rbs
	Comment 60 • 23 years ago

...And BTW, for the record, your question applies equally well to the other
prefs in the pres context... Possible gain in re-thinking the matter over there.
Perhaps, GetCachedBoolPref() and friends could just be wrappers around static
functions.

	Marc Attinasi
	Comment 62 • 23 years ago

Comment on attachment 54322 [details] [diff] [review]
v4 - logic switch for dbaron

sr=attinasi

	Gervase Markham [:gerv]
	Comment 63 • 23 years ago

attinasi, tingley: whoa there... 

For consistency with other prefs, surely this should be called
"browser.blink.enabled" ? This is not just an academic point; we want to keep
the pref namespace tidy and consistent.

Gerv

	Marc Attinasi
	Comment 64 • 23 years ago

Gerv, I saw consistency:

 pref("browser.anchor_color",                "#0000EE");
 pref("browser.visited_color",               "#551A8B");
 pref("browser.underline_anchors",           true);
+pref("browser.blink_allowed",               true);

How is 'browser.blink.enabled' any more consistent? To be honest I could go
either way, but the current pref looks fine to me, in keeping with similar pref
strings.

	Chase Tingley Assignee
	Comment 65 • 23 years ago

I chose the pref name based on claims that it was what NS4 used (see comment
from  Skewer 2001-08-26 13:58).  

I have no particular attachment to this name; I'll change it if given a good
candidate and a compelling reason.

	Gervase Markham [:gerv]
	Comment 66 • 23 years ago

I looked through all.js and it seems that most prefs of the "on/off" sort end
with ".enabled". But if the current pref is 4xp, that's slightly different. It's
probably not bothering with.

Gerv

	Chase Tingley Assignee
	Comment 67 • 23 years ago

I looked it up, and it appears to be a valid NS4 pref.

http://developer.netscape.com/docs/manuals/communicator/preferences/newprefb.html#browser_blink_allowed

	Marc Attinasi
	Comment 68 • 23 years ago

I'll check this in today then.

	Marc Attinasi
	Comment 69 • 23 years ago

Checking in nsHTMLReflowState.cpp;
/cvsroot/mozilla/layout/html/base/src/nsHTMLReflowState.cpp,v  <-- 
nsHTMLRefloState.cpp
new revision: 1.122; previous revision: 1.121
done

Checking in all.js;
/cvsroot/mozilla/modules/libpref/src/init/all.js,v  <--  all.js
new revision: 3.289; previous revision: 3.288
done

Backend changes checked into trunk. Thanks Chase!

	Chase Tingley Assignee
	Comment 70 • 23 years ago

Since at the most basic level, the accessibility requirement has been meant, I'm
going to mark this bug fixed.  There's the still the issue of UI control over
the pref -- I've spun off bug 106462 to deal with this.

	Skewer
	Comment 71 • 23 years ago

Verified 2001102403 W98 using attachment 38667 [details].

	Greg K.
	Comment 72 • 22 years ago

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

	Ruslan Ismailov
	Comment 73 • 22 years ago

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