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)

enhancement

Tracking

()

RESOLVED FIXED
mozilla1.4alpha

People

(Reporter: bwucke+bug, Assigned: dbaron)

References

()

Details

Attachments

(2 files)

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:
> 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...
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.
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 :-)
Priority: -- → P5
Target Milestone: --- → Future
Taking
Assignee: font → dbaron
Priority: P5 → P2
Target Milestone: Future → mozilla1.4alpha
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). :-)
BLINK has been around since Netscape 1. /be
*** Bug 213625 has been marked as a duplicate of this bug. ***
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
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); } }
> 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; ...
Attachment #128457 - Flags: superreview?(bzbarsky)
Attachment #128457 - Flags: review?(bzbarsky)
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 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+
Fix checked in to trunk, 2003-07-24 14:26 -0700.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
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.
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.

Attachment

General

Created:
Updated:
Size: