Closed
Bug 188761
Opened 22 years ago
Closed 22 years ago
Change visible:invisible times in <blink> (make it less evil)
Categories
(Core :: Layout: Text and Fonts, enhancement, P2)
Core
Layout: Text and Fonts
Tracking
()
RESOLVED
FIXED
mozilla1.4alpha
People
(Reporter: bwucke+bug, Assigned: dbaron)
References
()
Details
Attachments
(2 files)
42 bytes,
text/html; charset=UTF-8
|
Details | |
2.74 KB,
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3a) Gecko/20021212
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3a) Gecko/20021212
The worst problem with the <blink> tag is that the text disappears for
considerable amount of time, when you can't read it, and you lose track of the
point where you finished reading - before you find it anew, the text disappears
again. Bug #173540 suggests a pretty-looking solution, but it would probably
require much more computing power, quite a bit of code and I'm afraid the effect
wouldn't cause the blinking text stand out enough when it comes to important things.
My solution is much simpler - just make the text be visible most of the time and
disappear for a really short moment. Now it is visible for about half a second
and invisible for another half. Make it visible for 0.8s and invisible for 0.2s
(or something like this - a few experiments what is sufficient would help) -
This way the blinking still stands out strongly, but reading isn't so unpleasant
- you don't actually wait for the text to reappear and don't look for "lost
thread". From coding side this should require just changing parameters in
existing loops, without adding any new code.
Reproducible: Always
Steps to Reproduce:
![]() |
||
Comment 1•22 years ago
|
||
> From coding side this should require just changing parameters in
> existing loops, without adding any new code.
Nope. Since there are no loops. There is just a timer that fires at 750 ms
intervals and flips the text on and off. So it would either need to use
something other than a boolean for the blink state (eg values 0-4, text off for
0, on for the others, with the timer firing a lot more often) or you'd need to
write two separate timers and synch them somehow...
Fwiw, I think the idea of having the timer take on values in Z/n for some n,
having the timer fire every 1/n seconds and making sure to paint only when the
text actually needs to go off or come on may work...
Comment 2•22 years ago
|
||
I read somewhere that the US military standard for blinking was 3 cycles on for
every one cycle off. Someone should do some research into what the best timings
are to avoid seizures, etc.
Google didn't help me much.
Comment 3•22 years ago
|
||
According to Jamie Zawinski :
http://groups.google.com/groups?selm=31C46266.500F%40netscape.com&oe=UTF-8
I don't know if it's true, but it's funny at least :-)
Assignee | ||
Updated•22 years ago
|
Updated•22 years ago
|
Priority: -- → P5
Target Milestone: --- → Future
Assignee | ||
Comment 4•22 years ago
|
||
Taking
Assignee: font → dbaron
Priority: P5 → P2
Target Milestone: Future → mozilla1.4alpha
Comment 5•22 years ago
|
||
Since 1.4a is past, should we up the target milestone?
By the way, why does Netscape support BLINK anyway? I though it was a NS 4-only
proprietary extension to HTML, not a standard. I'm still searching Bugzilla for
that one (feel free to point me to the bug if it exists). :-)
Comment 6•22 years ago
|
||
BLINK has been around since Netscape 1.
/be
Comment 7•22 years ago
|
||
*** Bug 213625 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 8•22 years ago
|
||
Assignee | ||
Comment 9•22 years ago
|
||
FWIW, my tests in Netscape 4 (for Linux) and Netscape 3 (for Mac) show that the
timing there is 1/2 on, 1/2 off.
That agrees with the code in classic:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/lib/layout/laylayer.c&rev=3.9&mark=72,119-137,175-181#71
Reporter | ||
Comment 10•22 years ago
|
||
Wouldn't something like this do the trick?
(750:250 give the 3:1 military ratio. According to this, and JZW's comment
(referenced in comment #3) this bug becomes "normal", not RFE.)
#define LO_BLINK_RATE_SHOWN 750
#define LO_BLINK_RATE_HIDDEN 250
static void
lo_blink_callback(void *closure)
{
MWContext *context = (MWContext *)closure;
if (context->blink_hidden == FALSE) {
CL_HideLayerGroup(context->compositor, LO_BLINK_GROUP_NAME);
context->blink_hidden = TRUE;
context->blink_timeout = FE_SetTimeout(lo_blink_callback,
(void *)context,
LO_BLINK_RATE_HIDDEN);
} else {
CL_UnhideLayerGroup(context->compositor, LO_BLINK_GROUP_NAME);
context->blink_hidden = FALSE;
context->blink_timeout = FE_SetTimeout(lo_blink_callback,
(void *)context,
LO_BLINK_RATE_SHOWN);
}
}
Comment 11•22 years ago
|
||
> FWIW, my tests in Netscape 4 (for Linux) and Netscape 3 (for Mac) show
> that the timing there is 1/2 on, 1/2 off.
I guess that means it's been broken longer than I thought (damn you Aleks!)
NS3 cmd/xfe/lay.c line 2666:
/* Blinking. Oh the humanity. */
static int blink_state = 0;
static int blink_time = 250;
...
static void
fe_blink_timer (XtPointer closure, XtIntervalId *id)
{
...
fe_display_text (context, 0, b->text, 0, b->text->text_len,
(blink_state == 3));
...
blink_state = (blink_state + 1) % 4;
...
Assignee | ||
Comment 12•22 years ago
|
||
Assignee | ||
Updated•22 years ago
|
Attachment #128457 -
Flags: superreview?(bzbarsky)
Attachment #128457 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 13•22 years ago
|
||
Comment on attachment 128457 [details] [diff] [review]
patch to do 750 on, 250 off
>+ sState = (sState + 1) % 4;
>+ if (sState == 1 || sState == 2)
>+ // States 0, 1, and 2 are all the same.
>+ return;
Changed to |return NS_OK;| in my tree. I wish gcc would make that an error
like many other compilers...
![]() |
||
Comment 14•22 years ago
|
||
Comment on attachment 128457 [details] [diff] [review]
patch to do 750 on, 250 off
Looks good.
Attachment #128457 -
Flags: superreview?(bzbarsky)
Attachment #128457 -
Flags: superreview+
Attachment #128457 -
Flags: review?(bzbarsky)
Attachment #128457 -
Flags: review+
Assignee | ||
Comment 15•22 years ago
|
||
Fix checked in to trunk, 2003-07-24 14:26 -0700.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 16•22 years ago
|
||
And modification, at brendan's suggestion (use unsigned type) checked in as
well. Which also fixes the problem that caillon pointed out that I forgot to
change one of the PRBool's to PRInt32. Oops.
Comment 17•22 years ago
|
||
It's important to strength-reduce (x % power_of_two_constant) by hand via &, or
else make x have unsigned integer type and use a non-broken compiler.
Otherwise a sign test is required before the optimizer can emit the
strength-reduced code, and the negative case requires subtracting the modulus
from the & result (unless the compiler can analyze data flow and determine that
x is always non-negative).
/be
You need to log in
before you can comment on or make changes to this bug.
Description
•