Closed
Bug 19258
Opened 25 years ago
Closed 23 years ago
Blinking text must be optional through a user pref
Categories
(SeaMonkey :: General, defect, P3)
SeaMonkey
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla0.9.6
People
(Reporter: xiaotong, Assigned: tingley)
References
Details
(Keywords: access, arch, Whiteboard: relnote-user)
Attachments
(3 files, 2 obsolete files)
1.11 KB,
text/html
|
Details | |
3.13 KB,
patch
|
Details | Diff | Splinter Review | |
3.04 KB,
patch
|
dbaron
:
review+
attinasi
:
superreview+
|
Details | Diff | Splinter Review |
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.
Updated•25 years ago
|
Assignee: shuang → german
Comment 1•25 years ago
|
||
don't know whether it is possible to turn off animated or blink image within
browser. german, any idea?
I don't think this is possible, but let's check with the apps team: Don?
Assignee: german → don
Updated•25 years ago
|
Severity: normal → enhancement
Summary: allow user to turn on and off animation and blinking text/image → [RFE] allow user to turn on and off animation and blinking text/image
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.
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.
Moving all UE/UI bugs to new component: User Interface: Design Feedback
UE/UI component will be deleted.
Component: UE/UI → User Interface: Design Feedback
Updated•25 years ago
|
Target Milestone: M16 → Future
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?
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.
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.
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.
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.
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...)
Comment 14•24 years ago
|
||
Since Don has left, Vishy is taking his bugs in bulk, pending reassignment.
thanks,
Vishy
Assignee: don → vishy
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...
QA Contact: claudius → zach
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.
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.
Comment 18•24 years ago
|
||
That's no reason not to make it optional.
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".
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.
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...
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?
Comment 23•24 years ago
|
||
It was my understanding that this is a part of the accessibility effort.
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.
OS: Windows NT → All
Hardware: PC → All
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.
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.
Comment 27•24 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).
Comment 28•24 years ago
|
||
Comment 29•24 years ago
|
||
Hey, looks like someone's ahead of us. Blinking text appears to be disabled in
2001061504 Win98.
Comment 30•24 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.
Severity: enhancement → major
Keywords: fcc508
Comment 31•24 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.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
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.
Comment 33•23 years ago
|
||
aaron, that's covered in another bug
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).
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.
Keywords: nsCatFood
Summary: [RFE] allow user to turn on and off animation and blinking text/image → Blinking text must be optional through a user pref
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).
Keywords: 4xp
Comment 37•23 years ago
|
||
This bug isn't "fixed", there's no user pref. Reopening and adding dependency on
bug 89065.
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.
Assignee: vishy → tingley
Status: REOPENED → NEW
Assignee | ||
Updated•23 years ago
|
Assignee | ||
Comment 39•23 years ago
|
||
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.
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.
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.
Assignee | ||
Comment 43•23 years ago
|
||
Comment on attachment 53052 [details] [diff] [review]
patch - support for "browser.blink_allowed"
Good idea.
Attachment #53052 -
Attachment is obsolete: true
Assignee | ||
Comment 44•23 years ago
|
||
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
Attachment #53197 -
Flags: superreview+
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.
Keywords: nsenterprise
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?
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.
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.
Target Milestone: mozilla1.0 → mozilla0.9.6
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).
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.
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!
Attachment #53197 -
Attachment is obsolete: true
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.
Assignee | ||
Comment 54•23 years ago
|
||
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.
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
Comment 57•23 years ago
|
||
I wonder why there seems to be little use of the helpful
nsPresContext::GetCachedBoolPref(). It seems appropriate here.
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?
Comment 59•23 years ago
|
||
If that's what you intended then the patch does the job. Forget my comments.
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.
Assignee | ||
Comment 61•23 years ago
|
||
Attachment #54322 -
Flags: review+
Comment 62•23 years ago
|
||
Comment on attachment 54322 [details] [diff] [review]
v4 - logic switch for dbaron
sr=attinasi
Attachment #54322 -
Flags: superreview+
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
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.
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.
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
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
Comment 68•23 years ago
|
||
I'll check this in today then.
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!
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.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago → 23 years ago
Resolution: --- → FIXED
Comment 71•23 years ago
|
||
Verified 2001102403 W98 using attachment 38667 [details].
Comment 72•22 years ago
|
||
*** Bug 172897 has been marked as a duplicate of this bug. ***
Comment 73•22 years ago
|
||
*** Bug 190769 has been marked as a duplicate of this bug. ***
Updated•20 years ago
|
Product: Browser → Seamonkey
Comment 74•6 years ago
|
||
Keywords: sec508
Comment hidden (collapsed) |
You need to log in
before you can comment on or make changes to this bug.
Description
•