Last Comment Bug 857820 - Drop only blink effect from text-decoration: blink; and completely remove <blink> element
: Drop only blink effect from text-decoration: blink; and completely remove <bl...
Status: RESOLVED FIXED
: doc-bug-filed, site-compat
Product: Core
Classification: Components
Component: Layout: Text (show other bugs)
: Trunk
: All All
: -- enhancement with 4 votes (vote)
: mozilla23
Assigned To: Masayuki Nakano [:masayuki] (Mozilla Japan)
:
:
Mentors:
Depends on: 861756 861757 861760 861761 861763 910664
Blocks: 233150 812995 873257 873258
  Show dependency treegraph
 
Reported: 2013-04-03 15:34 PDT by Masayuki Nakano [:masayuki] (Mozilla Japan)
Modified: 2014-06-13 03:46 PDT (History)
33 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
23+


Attachments
part.1 Drop text-decoration: blink; support (51.17 KB, patch)
2013-04-04 05:03 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Splinter Review
part.2 Drop <blink> support from editor (12.77 KB, patch)
2013-04-04 07:56 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Splinter Review
part.3 Drop <blink> support from HTML parser (35.81 KB, patch)
2013-04-04 07:57 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
mrbkap: review+
Details | Diff | Splinter Review
part.1 Drop text-decoration: blink; support (19.37 KB, patch)
2013-04-04 20:00 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
dbaron: review+
Details | Diff | Splinter Review
part.1 Drop blink effect implementation r=dbaron (21.87 KB, patch)
2013-04-05 18:55 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Splinter Review
completely remove <blink> element (211.96 KB, patch)
2013-04-10 20:57 PDT, seth1103
no flags Details | Diff | Splinter Review
part.2 Drop <blink> support from editor (9.67 KB, patch)
2013-04-11 15:04 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
ehsan: review+
Details | Diff | Splinter Review
part.4 followup fix (remove NOISY_BLINK) (677 bytes, patch)
2013-04-17 18:10 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
dbaron: review+
Details | Diff | Splinter Review

Description Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-03 15:34:50 PDT
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.
Comment 1 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-03 16:23:46 PDT
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
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2013-04-03 20:00:04 PDT
I think we should nuke both, honestly.
Comment 3 David Baron :dbaron: ⌚️UTC-7 2013-04-03 20:21:10 PDT
I'd be fine with getting rid of both.  I don't think we should do a css-animation-based <blink>.
Comment 4 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-04 05:03:24 PDT
Created attachment 733263 [details] [diff] [review]
part.1 Drop text-decoration: blink; support

https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=1b296990ad76
Comment 5 Masatoshi Kimura [:emk] 2013-04-04 06:01:44 PDT
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
Comment 6 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-04 07:56:39 PDT
Created attachment 733326 [details] [diff] [review]
part.2 Drop <blink> support from editor
Comment 7 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-04 07:57:35 PDT
Created attachment 733328 [details] [diff] [review]
part.3 Drop <blink> support from HTML parser

https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=c3daa815a8d8
Comment 8 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-04 07:59:18 PDT
(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...
Comment 10 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-04 20:00:35 PDT
Created attachment 733713 [details] [diff] [review]
part.1 Drop text-decoration: blink; support

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.
Comment 11 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-04 20:04:28 PDT
Ah, so, "Drop blink effect implementation" is better for the summary of the part.1 patch.
Comment 12 Masatoshi Kimura [:emk] 2013-04-05 07:39:35 PDT
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.
Comment 13 Daniel Holbert [:dholbert] 2013-04-05 08:48:54 PDT
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"
Comment 14 Masatoshi Kimura [:emk] 2013-04-05 09:53:29 PDT
Hm, then the refest would be overkill.
Comment 15 Daniel Holbert [:dholbert] 2013-04-05 15:11:02 PDT
(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 16 David Baron :dbaron: ⌚️UTC-7 2013-04-05 15:26:38 PDT
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
Comment 17 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-05 16:43:08 PDT
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.
Comment 18 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-05 16:59:00 PDT
Comment on attachment 733326 [details] [diff] [review]
part.2 Drop <blink> support from editor

Also this.
Comment 19 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-05 18:55:28 PDT
Created attachment 734176 [details] [diff] [review]
part.1 Drop blink effect implementation r=dbaron
Comment 20 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-07 15:03:19 PDT
FYI: Blink won't implement <blink> with CSS Animation:
https://codereview.chromium.org/13723005/
Comment 21 Blake Kaplan (:mrbkap) 2013-04-08 11:25:27 PDT
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.
Comment 22 Blake Kaplan (:mrbkap) 2013-04-08 11:26:21 PDT
Comment on attachment 733326 [details] [diff] [review]
part.2 Drop <blink> support from editor

Ehsan should really make the call here.
Comment 23 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-08 11:32:40 PDT
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
Comment 24 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-08 11:40:56 PDT
(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 25 :Ehsan Akhgari 2013-04-08 13:19:26 PDT
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...
Comment 26 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-08 15:48:21 PDT
(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.
Comment 27 Masatoshi Kimura [:emk] 2013-04-08 16:18:29 PDT
(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.
Comment 28 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-08 17:47:22 PDT
(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.
Comment 29 Masatoshi Kimura [:emk] 2013-04-08 18:37:21 PDT
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").
Comment 30 Aryeh Gregor (:ayg) (away until October 25) 2013-04-09 04:43:25 PDT
(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.
Comment 31 Daniel Glazman (:glazou) 2013-04-09 07:41:28 PDT
(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...
Comment 32 Masatoshi Kimura [:emk] 2013-04-09 08:10:21 PDT
(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.
Comment 33 :Ehsan Akhgari 2013-04-09 08:11:55 PDT
(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.
Comment 34 :Ehsan Akhgari 2013-04-09 08:14:38 PDT
(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>.)
Comment 35 Masatoshi Kimura [:emk] 2013-04-09 08:22:52 PDT
(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
Comment 36 Daniel Glazman (:glazou) 2013-04-09 09:18:11 PDT
(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'.
Comment 37 Masatoshi Kimura [:emk] 2013-04-09 09:20:35 PDT
(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?
Comment 38 Daniel Glazman (:glazou) 2013-04-09 09:36:57 PDT
(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.
Comment 39 Masatoshi Kimura [:emk] 2013-04-09 09:52:15 PDT
(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?
Comment 40 Masatoshi Kimura [:emk] 2013-04-09 09:55:57 PDT
Also note that CSS 2.1 is already the Recommendation. Why it is too early to follow now?
Comment 41 Masatoshi Kimura [:emk] 2013-04-10 00:45:09 PDT
(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.
Comment 42 Eric A. Meyer 2013-04-10 07:40:40 PDT
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.
Comment 43 Masatoshi Kimura [:emk] 2013-04-10 08:27:00 PDT
(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...
Comment 44 Masatoshi Kimura [:emk] 2013-04-10 08:42:21 PDT
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.
Comment 45 Eric A. Meyer 2013-04-10 09:54:06 PDT
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 46 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-10 15:27:50 PDT
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.
Comment 47 Blake Kaplan (:mrbkap) 2013-04-10 15:45:16 PDT
Comment on attachment 733328 [details] [diff] [review]
part.3 Drop <blink> support from HTML parser

Sure. Sorry for the confusion.
Comment 48 seth1103 2013-04-10 20:57:36 PDT
Created attachment 736112 [details] [diff] [review]
completely remove <blink> element
Comment 49 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-11 15:04:20 PDT
Created attachment 736517 [details] [diff] [review]
part.2 Drop <blink> support from editor

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.
Comment 50 :Ehsan Akhgari 2013-04-11 16:36:38 PDT
Ms2ger, do  you know how we can get the spec tests updated for this?
Comment 51 :Ms2ger (⌚ UTC+1/+2) 2013-04-11 23:54:08 PDT
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.
Comment 52 Aryeh Gregor (:ayg) (away until October 25) 2013-04-12 04:11:32 PDT
(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.
Comment 53 :Ehsan Akhgari 2013-04-12 13:42:24 PDT
(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.
Comment 54 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-13 15:17:32 PDT
(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.
Comment 57 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-14 21:20:29 PDT
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.
Comment 58 Kohei Yoshino [:kohei] 2013-04-15 07:11:22 PDT
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
Comment 59 Anne (:annevk) 2013-04-15 10:28:08 PDT
(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?
Comment 60 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-15 13:24:42 PDT
(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...
Comment 61 Masatoshi Kimura [:emk] 2013-04-15 15:57:08 PDT
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 62 Anne (:annevk) 2013-04-16 02:27:08 PDT
Filed https://www.w3.org/Bugs/Public/show_bug.cgi?id=21712 Thanks Masatoshi.
Comment 63 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-17 18:10:04 PDT
Created attachment 738838 [details] [diff] [review]
part.4 followup fix (remove NOISY_BLINK)
Comment 64 David Baron :dbaron: ⌚️UTC-7 2013-04-23 12:48:09 PDT
Comment on attachment 738838 [details] [diff] [review]
part.4 followup fix (remove NOISY_BLINK)

r=dbaron
Comment 65 Cngevpxhaqrefpber 2013-04-23 19:53:11 PDT
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.
Comment 66 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-04-23 20:08:05 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/1d6d1136ee15
Comment 67 Ryan VanderMeulen [:RyanVM] 2013-04-24 05:22:19 PDT
https://hg.mozilla.org/mozilla-central/rev/1d6d1136ee15
Comment 68 Paul Irish 2013-04-24 12:43:39 PDT
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.
Comment 69 Eric A. Meyer 2013-04-24 12:50:55 PDT
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.
Comment 70 David Baron :dbaron: ⌚️UTC-7 2013-04-24 13:02:35 PDT
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.
Comment 71 P. Envall 2013-04-24 13:27:54 PDT
I think you just broke all geocities pages.

Must keep a legacy browser around to experience them now.
Comment 72 Sindre Jensen 2013-05-09 03:41:42 PDT
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!
Comment 73 Lukas Blakk [:lsblakk] use ?needinfo 2013-05-21 11:13:52 PDT
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.
Comment 74 rexyrexy2 2013-06-28 07:58:50 PDT
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.
Comment 75 rexyrexy2 2013-06-28 08:06:46 PDT
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.
Comment 76 rexyrexy2 2013-06-28 08:15:10 PDT
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)?
Comment 77 Kris Maglione [:kmag] 2013-08-20 14:57:25 PDT
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.

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