Closed Bug 857820 Opened 11 years ago Closed 11 years ago

Drop only blink effect from text-decoration: blink; and completely remove <blink> element

Categories

(Core :: Layout: Text and Fonts, enhancement)

Product:
Core
Component:
Layout: Text and Fonts
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla23
Tracking Flags:
Tracking Status
relnote-firefox --- 23+

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: doc-bug-filed, site-compat)

Attachments

(4 files, 4 obsolete files)

part.3 Drop <blink> support from HTML parser
35.81 KB, patch
mrbkap
: review+
Details | Diff | Splinter Review
part.1 Drop blink effect implementation r=dbaron
21.87 KB, patch
Details | Diff | Splinter Review
part.2 Drop <blink> support from editor
9.67 KB, patch
ehsan.akhgari
: review+
Details | Diff | Splinter Review
part.4 followup fix (remove NOISY_BLINK)
677 bytes, patch
dbaron
: review+
Details | Diff | Splinter Review 
This is not decision, this is just my suggestion.

We had a very bad news, Opera will switch its engine to WebKit.

Then, Gecko will be the only one, it supports blink effect.

So that I think that there is no reason we keep supporting the blink.

First, blink is now marked as "deprecated" in the draft of CSS3 Text Decoration. The draft recommends CSS3 Animation instead. The blink effect is available only with text node, but CSS3 Animation is available any elements. This is very big advantage.

Next, even if we drop the blink effect, we don't need to worry about the compatibility with older Gecko because any other browsers don't support it after Presto retires. Additionally, in these days, blink is not major feature due to its a11y issue.

Finally, our implementation is not beautiful.
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsTextFrameThebes.cpp#3257
We're using timer for implementing it. If we can remove it, it's great.
Even if we drop support text-decoration: blink, we can keep to supporting only <blink> element with default style sheet. For example:

http://www.john-smith.me/emulating--lt-blink-gt--using-webkit-css3-animation
I think we should nuke both, honestly.
I'd be fine with getting rid of both.  I don't think we should do a css-animation-based <blink>.
We shouldn't remove "blink" keyword even if we don't support the blink effect. Both IE10 and Chrome recognize "text-decoration: underline blink;".
Per CSS 2.1, conforming user agents are allowed not to blink the text.
http://www.w3.org/TR/CSS21/text.html#lining-striking-props
(In reply to Masatoshi Kimura [:emk] from comment #5)
> We shouldn't remove "blink" keyword even if we don't support the blink
> effect. Both IE10 and Chrome recognize "text-decoration: underline blink;".
> Per CSS 2.1, conforming user agents are allowed not to blink the text.
> http://www.w3.org/TR/CSS21/text.html#lining-striking-props

Ah...
Assignee: nobody → masayuki
Currently, mozilla-central is for 23 which will be released at early of August. I think that it's not so bad timing for removing blink support. If you don't think so, please give r- for this patch with your idea of the timing.

This patch only drops the blink effect. The style system keeps having the blink keyword support for compatibility with other browsers and DOM property values.
See comment 5 why I chose this approach.

If you agree with this patch, I'll ask whether we need to remove blink element support from HTML parser or not to somebody of HTML parser peer.
Attachment #733263 - Attachment is obsolete: true
Attachment #733713 - Flags: review?(dbaron)
Ah, so, "Drop blink effect implementation" is better for the summary of the part.1 patch.
Comment on attachment 733713 [details] [diff] [review]
part.1 Drop text-decoration: blink; support

Please add a reftest to make sure that "text-decoration: underline blink;" doesn't regress.
At a purely parser level, we already have tests for that, FWIW (to be sure that sort of thing is accepted as a valid text-decoration value):
https://mxr.mozilla.org/mozilla-central/source/layout/style/test/property_database.js?rev=e7a9e30409eb&mark=3108-3108#3102

Note in particular "blink line-through underline", "underline overline line-through blink"
Hm, then the refest would be overkill.
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #0)
> Then, Gecko will be the only one, it supports blink effect.
> 
> So that I think that there is no reason we keep supporting the blink.

FWIW, there's an issue filed for adding 'blink' support to blink-the-rendering-engine:
 https://codereview.chromium.org/13723005

So we may not be alone after all. (or alternately, if we do remove blink support, then blink-the-new-rendering-engine may end up being the only one to support it, which would be kind of hilarious/awesome. :))
Comment on attachment 733713 [details] [diff] [review]
part.1 Drop text-decoration: blink; support

Please also remove this:
#define TEXT_BLINK_ON NS_FRAME_STATE_BIT(29)
near the top of nsTextFrameThebes.cpp, and the one reference to it in a comment.

r=dbaron with that
Attachment #733713 - Flags: review?(dbaron) → review+
Comment on attachment 733328 [details] [diff] [review]
part.3 Drop <blink> support from HTML parser

I'm not sure whether we should remove blink "tag" support from parser too. Should we keep to support it for the compatibility or other reasons? If so, please mark r-. Then, I'll land only the part.1 patch.

Note that Chromium might support <blink> with CSS Animation. See comment 15. So, it might be necessary if they will support it.
Attachment #733328 - Flags: review?(mrbkap)
Comment on attachment 733326 [details] [diff] [review]
part.2 Drop <blink> support from editor

Also this.
Attachment #733326 - Flags: review?(mrbkap)
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Keywords: dev-doc-needed
FYI: Blink won't implement <blink> with CSS Animation:
https://codereview.chromium.org/13723005/
Comment on attachment 733328 [details] [diff] [review]
part.3 Drop <blink> support from HTML parser

It's probably better to just let this stuff be. I don't think this patch actually affects any behavior anymore, given that we've switched over to the HTML5 parser.
Attachment #733328 - Flags: review?(mrbkap) → review-
Comment on attachment 733326 [details] [diff] [review]
part.2 Drop <blink> support from editor

Ehsan should really make the call here.
Attachment #733326 - Flags: review?(mrbkap) → review?(ehsan)
Comment on attachment 733326 [details] [diff] [review]
part.2 Drop <blink> support from editor

mrbkap:

Thank you for your review. I don't think we need the ehsan's review for this because this patch does break a lot of
Attachment #733326 - Flags: review?(ehsan) → feedback?(ehsan)
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #23)
> Comment on attachment 733326 [details] [diff] [review]
> part.2 Drop <blink> support from editor
> 
> mrbkap:
> 
> Thank you for your review. I don't think we need the ehsan's review for this
> because this patch does break a lot of

Oops, here is the following comment:

because this patch does break a lot of test failure. So, it means we need to keep the <blink> "element" edit feature in HTML editor. I don't think it's bad thing in this case (part.3 won't be landed).

1. We actually support <blink> element. Although, the blink effect is killed.
2. Keeping <blink> element support in editor provides better compatibility with any documents generated by older Gecko.

Now, mrbkap has decided to keep supporting <blink> tag support in legacy parser, then, I think that it does make sense that we keep <blink> element edit support.

However, I'd like to know Ehsan's suggestion if he has something about it. So, I moved the flag from r? to feedback?.
Comment on attachment 733326 [details] [diff] [review]
part.2 Drop <blink> support from editor

Review of attachment 733326 [details] [diff] [review]:
-----------------------------------------------------------------

Daniel, does removing <blink> support from the editor affect your use of it?  I don't necessarily think that these tests failures are something we should worry about, since the editing spec should not really be mentioning <blink> as an inline element in the first place...
Attachment #733326 - Flags: feedback?(ehsan) → feedback?(daniel)
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #25)
> Daniel, does removing <blink> support from the editor affect your use of it?  I
> I don't necessarily think that these tests failures are something we should
> worry about, since the editing spec should not really be mentioning <blink>
> as an inline element in the first place...

FYI: If we land part.2 patch without part.3, there are a lot of test failures:
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=841ba1bec57f

So, my question is, "Leaving <blink> element support on editor doesn't cause any problem?". Be careful, not trying to get approval of removing the support.

In other words, I'm trying to land only part.1 patch. Currently, I have no plan to take part.2 and part.3 onto m-c.

If you misunderstood my intention, I apologize about not enough explanation for you at requesting the feedback.
Flags: needinfo?(ehsan)
Summary: Drop text-decoration: blink; and <blink> support → Drop only blink effect from text-decoration: blink; and <blink> element
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #26)
> FYI: If we land part.2 patch without part.3, there are a lot of test
> failures:
> https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=841ba1bec57f

Then the following comment is wrong, no?
(In reply to Blake Kaplan (:mrbkap) from comment #21)
> I don't think this patch
> actually affects any behavior anymore, given that we've switched over to the
> HTML5 parser.

It obviously affects the behavior.
(In reply to Masatoshi Kimura [:emk] from comment #27)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #26)
> > FYI: If we land part.2 patch without part.3, there are a lot of test
> > failures:
> > https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=841ba1bec57f
> 
> Then the following comment is wrong, no?
> (In reply to Blake Kaplan (:mrbkap) from comment #21)
> > I don't think this patch
> > actually affects any behavior anymore, given that we've switched over to the
> > HTML5 parser.
> 
> It obviously affects the behavior.

Hmm, I feel strange for "we've switched over to the HTML5 parser".

However, note that both part.2 and part.3 patch remove blink atom definitions which were copied from one of them. Therefore, only removing the editor part caused something mismatch and makes some tests orange. So, part.2 and part.3 should be one patch technically. Therefore, it causes the orange at landing only part.2 and the orange is fixed by part.3. Then, it looks like part.3 patch changes the behavior actually.

So, anyway, I also don't think that the result will be changed even if part.2 and part.3 are landed or not.
Even if both part 2 and part 3 landed, there is an observable behavior change.
< document.createElement('blink')
> [object HTMLElement] (without part 2 & 3)
> [object HTMLUnknownElement] (with part 2 & 3)
The latter is correct per the spec.
I think the decision should be reconsidered because it is based on the wrong assumption ("I don't think this patch anymore").
Flags: needinfo?(mrbkap)
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #25)
> Daniel, does removing <blink> support from the editor affect your use of it?
> I don't necessarily think that these tests failures are something we should
> worry about, since the editing spec should not really be mentioning <blink>
> as an inline element in the first place...

No changes should be made to dom/imptests/editing/ -- they'll be overwritten when we sync next from upstream.  The changes have to be made in the source repository.  Or, add new expected failures in the appropriate JSON file in dom/imptests/failures/editing/.

I don't have a problem in principle with removing <blink> support from the editing spec, but I'm not actively maintaining the spec anymore.  Probably some other browsers should be tested to double-check that we won't be reducing interop.
Keywords: site-compat
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #25)

> Daniel, does removing <blink> support from the editor affect your use of it?
> I don't necessarily think that these tests failures are something we should
> worry about, since the editing spec should not really be mentioning <blink>
> as an inline element in the first place...

This question is irrelevant. The editor - and BlueGriffon above it - will
do what's allowed by Web Standards. If Web Standards allow 'text-decoration:
blink', so be it. CSS 3 Text Decoration is only a Working Draft at this time,
and CSS 3 Animation are a WD too. So, in principle, this bug does the right
thing, but it does it too early, that's all.
With my CSS WG co-chairman's hat on, 'text-decoration: blink' is, for the time
being, still a CSS 2.1 provision and NOT deprecated. I recommend getting rid
of blink when Text Decoration reaches CR, if the deprecation is not itself
at-risk because Animations have not moved along the REC track...
(In reply to Daniel Glazman (:glazou) from comment #31)
> This question is irrelevant. The editor - and BlueGriffon above it - will
> do what's allowed by Web Standards. If Web Standards allow 'text-decoration:
> blink', so be it. CSS 3 Text Decoration is only a Working Draft at this time,
> and CSS 3 Animation are a WD too. So, in principle, this bug does the right
> thing, but it does it too early, that's all.
> With my CSS WG co-chairman's hat on, 'text-decoration: blink' is, for the
> time
> being, still a CSS 2.1 provision and NOT deprecated. I recommend getting rid
> of blink when Text Decoration reaches CR, if the deprecation is not itself
> at-risk because Animations have not moved along the REC track...

This patch (part 1) doesn't remove "text-decoration: blink;" from the syntax and DOM (CSSOM). It will just disable the blinking effect which is already allowed by CSS 2.1. See comment #5.
On the other hand, the HTML <blink> tag has never been spec'ed by the HTML standard. So it should be completely removed to much the spec. In other words, part 2 & 3 should be landed.
(In reply to Daniel Glazman (:glazou) from comment #31)
> (In reply to :Ehsan Akhgari (needinfo? me!) from comment #25)
> 
> > Daniel, does removing <blink> support from the editor affect your use of it?
> > I don't necessarily think that these tests failures are something we should
> > worry about, since the editing spec should not really be mentioning <blink>
> > as an inline element in the first place...
> 
> This question is irrelevant. The editor - and BlueGriffon above it - will
> do what's allowed by Web Standards. If Web Standards allow 'text-decoration:
> blink', so be it. CSS 3 Text Decoration is only a Working Draft at this time,
> and CSS 3 Animation are a WD too. So, in principle, this bug does the right
> thing, but it does it too early, that's all.

I was mostly talking about the <blink> tag, since that's what attachment 733326 [details] [diff] [review] is removing support for.
Flags: needinfo?(ehsan)
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #26)
> (In reply to :Ehsan Akhgari (needinfo? me!) from comment #25)
> > Daniel, does removing <blink> support from the editor affect your use of it?  I
> > I don't necessarily think that these tests failures are something we should
> > worry about, since the editing spec should not really be mentioning <blink>
> > as an inline element in the first place...
> 
> FYI: If we land part.2 patch without part.3, there are a lot of test
> failures:
> https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=841ba1bec57f
> 
> So, my question is, "Leaving <blink> element support on editor doesn't cause
> any problem?". Be careful, not trying to get approval of removing the
> support.

If the goal of this bug was morphed to disable <blink> elements from blinking at all, then leaving the support in the editor will not cause any harm (they will be treated similar to other inline elements, such as <span>.)
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #34)
> If the goal of this bug was morphed to disable <blink> elements from
> blinking at all,

The aim of this bug is BOTH text-decoration: blink and the <blink> element from the start. See the activity log.
https://bugzilla.mozilla.org/show_activity.cgi?id=857820
(In reply to Masatoshi Kimura [:emk] from comment #35)
> (In reply to :Ehsan Akhgari (needinfo? me!) from comment #34)
> > If the goal of this bug was morphed to disable <blink> elements from
> > blinking at all,
> 
> The aim of this bug is BOTH text-decoration: blink and the <blink> element
> from the start. See the activity log.
> https://bugzilla.mozilla.org/show_activity.cgi?id=857820

That's what I figured, yes. So I agree with removing <blink>, disagree with
removing 'text-decoration: blink'.
(In reply to Daniel Glazman (:glazou) from comment #36)
> (In reply to Masatoshi Kimura [:emk] from comment #35)
> > (In reply to :Ehsan Akhgari (needinfo? me!) from comment #34)
> > > If the goal of this bug was morphed to disable <blink> elements from
> > > blinking at all,
> > 
> > The aim of this bug is BOTH text-decoration: blink and the <blink> element
> > from the start. See the activity log.
> > https://bugzilla.mozilla.org/show_activity.cgi?id=857820
> 
> That's what I figured, yes. So I agree with removing <blink>, disagree with
> removing 'text-decoration: blink'.

Then the bug summary is modified so that it only disables the blinking effect without removing "text-decoration: blink". Do you disagree with it? If you do, why?
(In reply to Masatoshi Kimura [:emk] from comment #37)

> Then the bug summary is modified so that it only disables the blinking
> effect without removing "text-decoration: blink". Do you disagree with it?
> If you do, why?

AFAICT, the title is still about both; if you tweak it so only <blink>
is affected, I withdraw my objection of course. Please also note I said
I will eventually agree with getting rid of 'text-decoration: blink' but
just not right now since it is still in a Web Standards and not deprecated.
(In reply to Daniel Glazman (:glazou) from comment #38)
> (In reply to Masatoshi Kimura [:emk] from comment #37)
> 
> > Then the bug summary is modified so that it only disables the blinking
> > effect without removing "text-decoration: blink". Do you disagree with it?
> > If you do, why?
> 
> AFAICT, the title is still about both; if you tweak it so only <blink>
> is affected, I withdraw my objection of course. Please also note I said
> I will eventually agree with getting rid of 'text-decoration: blink' but
> just not right now since it is still in a Web Standards and not deprecated.

The current summary intended to indicate that we are only disabling the blinking effect. (If it is not clear enough, I'll gladly update the summary.) It is allowed by the CSS 2.1 spec and our browser is still conform to the spec even if we disabled the blinking effect from "text-decoration: blink".
Apparently you are too lazy to read the linked URL, so I'm quoting here:
> blink
>     Text blinks (alternates between visible and invisible). Conforming user agents may simply not blink the text. Note that not blinking the text is one technique to satisfy checkpoint 3.3 of WAI-UAAG.

I know you disagree with removing "text-decoration: blink" completely. Do you also disagree with removing only the blinking effect from the "text-decoration: blink"? And if you disagree the latter, why do you disagree with what the current spec explicitly allows?
Also note that CSS 2.1 is already the Recommendation. Why it is too early to follow now?
See Also: → 860140
(In reply to Blake Kaplan (:mrbkap) from comment #21)
> Comment on attachment 733328 [details] [diff] [review]
> part.3 Drop <blink> support from HTML parser
> 
> It's probably better to just let this stuff be. I don't think this patch
> actually affects any behavior anymore, given that we've switched over to the
> HTML5 parser.

HTML5 parser and document.createElement will end up with calling NS_NewHTMLElement which uses nsHTMLTagList.h to determine if a given tag name is known.
https://mxr.mozilla.org/mozilla-central/source/content/html/document/src/nsHTMLContentSink.cpp?rev=9eb2df812835#462
https://mxr.mozilla.org/mozilla-central/source/content/html/document/src/nsHTMLContentSink.cpp?rev=9eb2df812835#115
So changing nsHTMLTagList.h will affect the behavior even if other parts of the old parser is not used at all. And I actually confirned the behavior change.
On the CSS side alone (that is, I’m not commenting one way or the other about the HTML <blink> element)…

All other browsers I know of parse 'text-decoration: blink' and then don’t visually render the blinking; this should be the absolute minimum level of support in Gecko, at least until such time as the WG drops 'blink' from a future specification.  (And even then I'd argue to keep the parsing support for legacy reasons, though not very strongly.)

The only reason I can think to drop parsing support is if you want to give authors a Gecko-specific CSS hack based on '@supports', and I really don't think anyone wants to go there.  I hope not, anyway.
(In reply to Eric A. Meyer from comment #42)
> All other browsers I know of parse 'text-decoration: blink' and then don’t
> visually render the blinking; this should be the absolute minimum level of
> support in Gecko,

That's exactly what Gecko is trying to do at the moment.

> at least until such time as the WG drops 'blink' from a
> future specification.  (And even then I'd argue to keep the parsing support
> for legacy reasons, though not very strongly.)

I'm not saying about the CSS 3 draft at all, but saying about CSS 2.1, the current Recommendation.

It was I who said something like "we shouldn't drop parsing 'text-decoration: blink' because of the compatibility with other browsers and compliance to the spec" in this bug (comment #5). I don't understand why I have to answer the similar question over and over...
To summarize:
1. Regarding to the CSS, this bug was opened to drop parsing 'text-decoration: blink', but it was withdrawn at the point of comment #10. Because it will violate the spec and break the compatibility with other browsers.
2. Instead, this bug is trying to disable only the visual effect. CSS 2.1, the current Recommendation, already allows that explicitly.
3. Regarding to HTML, this bug was opened to drop the <blink> tag support. But the request has been rejected based on the false assumption. Although Gecko has switched to the HTML5 parser, it (and layout) still depends on a file in the parser/htmlparser directory.
I wasn't actually replying to you personally, Masatoshi—thus the lack of quoted text—but if you want to know why you keep having to answer again and again, the bug has a confusing comment history, a title that’s unclear at first glance, and context that many will not be aware of even if they closely read every single comment.

Your summary comment goes a long way toward addressing those things, so thank you for that.  A title more along the lines of "Block blinking of content triggered by text-decoration: blink and <blink>" might also help, unless "block" has a Bugzilla-specific meaning that I'm unaware of or have forgotten in the last decade.
Comment on attachment 733328 [details] [diff] [review]
part.3 Drop <blink> support from HTML parser

Thank you, Kimura-san. According to comment 29 and comment 44, I agree with we should remove <blink> support from parser and editor too.

So, this patch should be reviewed again by mrbkap.

Blake Kaplan, I think that we should remove the <blink> for web standards. This patch (and part.2) are necessary according to comment 29 and comment 44. Therefore, if you thought that this patch still were not needed, you should explain the reason.  Otherwise, please review the contents of the patch normally.
Attachment #733328 - Flags: review- → review?(mrbkap)
Comment on attachment 733328 [details] [diff] [review]
part.3 Drop <blink> support from HTML parser

Sure. Sorry for the confusion.
Attachment #733328 - Flags: review?(mrbkap) → review+
Flags: needinfo?(mrbkap)
Summary: Drop only blink effect from text-decoration: blink; and <blink> element → Drop only blink effect from text-decoration: blink; and completely remove <blink> element

    
    

    
  


  
Okay, let's remove <blink> support from editor too for completely removing <blink> element support by part.3.

And this is modified the dom/imptests part from the previous patch.

        Attachment #733326 -
        Attachment is obsolete: true

        Attachment #733326 -
        Flags: feedback?(daniel)

        Attachment #736517 -
        Flags: review?(ehsan)

        Attachment #736517 -
        Flags: review?(ayg)

    
  

        Attachment #736517 -
        Flags: review?(ehsan)

        Attachment #736517 -
        Flags: review?(ayg)

        Attachment #736517 -
        Flags: review+

    
    

    
  
Ms2ger, do  you know how we can get the spec tests updated for this?
Flags: needinfo?(Ms2ger)

    
    

    
  
They come from <https://dvcs.w3.org/hg/editing>. I probably have access to push there, but I'd prefer Aryeh to at least sign off the changes.
Flags: needinfo?(Ms2ger)

    
    

    
  
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #50)
> Ms2ger, do  you know how we can get the spec tests updated for this?

The spec tests should only be updated in sync with the test, which currently has no active editor.  My recommendation for now would be to add an expected failure to our .json file -- it won't be the only one that's there because we want the spec to change.

    
    

    
  
(In reply to comment #52)
> (In reply to :Ehsan Akhgari (needinfo? me!) from comment #50)
> > Ms2ger, do  you know how we can get the spec tests updated for this?
> 
> The spec tests should only be updated in sync with the test, which currently
> has no active editor.  My recommendation for now would be to add an expected
> failure to our .json file -- it won't be the only one that's there because we
> want the spec to change.

OK, that's what the patch here does AFAICT.

    
    

    
  
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #53)
> (In reply to comment #52)
> > (In reply to :Ehsan Akhgari (needinfo? me!) from comment #50)
> > > Ms2ger, do  you know how we can get the spec tests updated for this?
> > 
> > The spec tests should only be updated in sync with the test, which currently
> > has no active editor.  My recommendation for now would be to add an expected
> > failure to our .json file -- it won't be the only one that's there because we
> > want the spec to change.
> 
> OK, that's what the patch here does AFAICT.

Yes.

Thank you all guys who commented in this bug. I'll land them tonight (JST, +9:00) if nobody stops me doing that.

    
    

    
  
I should summarize what we changed:

1. Kept supporting text-decoration: blink; at CSS parser level and DOM API level.
2. Dropped blinking effect code from layout/.
3. Dropped <blink> element support from HTML parser. I.e., <blink> tags is parsed as unknown HTML element.

Note that the changes are not break any Web Standards. Not supporting blinking effect for text-decoration: blink; is allowed by CSS 2.1 and CSS 3 Text Decoration. And also, <blink> element isn't defined any Web Standard specs.

    
  

        Attachment #736112 -
        Attachment is obsolete: true

    
    

    
  
I've added this bug to the compatibility doc. Please correct the info if wrong.
https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_23

    
    

    
  
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #57)
> And also, <blink> element isn't defined any Web Standard specs.

FWIW, HTML does require `text-decoration: blink` at the moment. It does not require special parsing behavior however, was that something in the old parser?

    
    

    
  
(In reply to Kohei Yoshino from comment #58)
> I've added this bug to the compatibility doc. Please correct the info if
> wrong.
> https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_23

Thanks, looks good for me.

(In reply to Anne (:annevk) from comment #59)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #57)
> > And also, <blink> element isn't defined any Web Standard specs.
> 
> FWIW, HTML does require `text-decoration: blink` at the moment. It does not
> require special parsing behavior however, was that something in the old
> parser?

I'm not sure what you meant...

    
    

    
  
I guess Anne says the default style defined in HTML5 has the following line:

  blink { text-decoration: blink; }

|getComputedStyle(document.createElement('blink')).textDecoration| returned:
"blink" on Firefox 20,
"" on Chrome, and
"none" on IE10 and Nightly.
I think we should file a spec bug to remove the default style.

    
    

    
  
Comment on attachment 738838 [details] [diff] [review]
part.4 followup fix (remove NOISY_BLINK)

r=dbaron

        Attachment #738838 -
        Flags: review?(dbaron) → review+

    
    

    
  
I realize my saying anything is unlikely to change anything, and that's fine, since I don't *really* care too much either way, but I have to ask: what harm is there in continuing to support the <blink> tag? Just 'cause everyone else doesn't support it anymore doesn't necessarily mean that Firefox has to follow suit, does it?

It's been a part of the web for so long that dropping it just feels kind of... wrong.

    
    

    
  
As pointed out in http://meyerweb.com/eric/thoughts/2013/04/24/blink-supports/ it seems .. "surprising" that the spec indicates a conforming user agent would support the blink keyword by not blinking at all.

Regardless of the spec's ambiguous wording, I would expect the mozilla implementation to not pass a @supports() check.

    
    

    
  
That’s not what I was pointing out, Paul.  What I was pointing out was that if (as I argued a month back) @supports is really @parses, which I was assured that it is and should be, then it should be called that.

What’s happening in Firefox 23 and Chrome and so on is that 'text-decoration: blink' is recognized and parsed, so it meet the criteria for "@supports", and the text is thus made green on yellow with no blinking.  Only Firefox 22 blinks the green text on yellow background because it supports conditionals but didn't switch off blinking.

    
    

    
  
The 'blink' value of 'text-decoration' has been an exception to CSS's rules on "parse it only if you support it" since CSS1.  It's a one-off exception, and it should stay that way.

    
    

    
  
I think you just broke all geocities pages.

Must keep a legacy browser around to experience them now.

    
    

    
  
There is really no reason to remove <blink>, it has been part of Firefox for so long, and if that is what the creator of the HTML-code is writing, then why not let the text blink? It is hardly used anyway.

At least keep browser.blink_allowed so we can turn it on again!

    
  
Blocks: 873257

    
  
Blocks: 873258

    
  
Keywords: dev-doc-needed → doc-bug-filed
Blocks: 233150

    
    

    
  
This has been noted in the Aurora 23 release notes:

http://www.mozilla.org/en-US/firefox/23.0a2/auroranotes/

If you would like to make any changes or have questions/concerns please contact me directly.

    
    

    
  
It should still be in about:config, just like many things are, including support for html5. so there should be SOME way to get it back via about:config, you just should set the about:config entry to false by default.

    
    

    
  
you should add a browser.blink.effects_allowed or browser.blink_effects_allowed or browser.blinkeffects_allowed which is set to false by default, to disable or enable the effects.

    
    

    
  
Plus, chrome has nonstandards of its own, like the pop out of the side of the browser things...and do you see people complaining?
therefore, there is nothing wrong with having support for nonstandards.
plus, is there ANY good to come out of this, other than 'standards compliance' (which is not a reason to do this IMO)?

    
    

    
  
I owe everyone involved with this bug a beer. In fact, anyone who meets me in a bar and mentions this bug is entitled to one free beer of their choosing.

    
  
Depends on: 910664

          You need to log in
          before you can comment on or make changes to this bug.