Change visible:invisible times in <blink> (make it less evil)

RESOLVED FIXED in mozilla1.4alpha

Status

()

enhancement
P2
normal
RESOLVED FIXED
17 years ago
16 years ago

People

(Reporter: bwucke+bug, Assigned: dbaron)

Tracking

Trunk
mozilla1.4alpha
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

()

Attachments

(2 attachments)

Reporter

Description

17 years ago
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.

Comment 3

17 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 :-)
Priority: -- → P5
Target Milestone: --- → Future
Taking
Assignee: font → dbaron
Priority: P5 → P2
Target Milestone: Future → mozilla1.4alpha

Comment 5

16 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). :-)
BLINK has been around since Netscape 1.

/be

Comment 7

16 years ago
*** 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
Reporter

Comment 10

16 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

16 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;
    ...


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: 16 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.