The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla23

Status

()

Core
Layout: Text
--
enhancement
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

({doc-bug-filed, site-compat})

Trunk
mozilla23
doc-bug-filed, site-compat
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(relnote-firefox 23+)

Details

Attachments

(4 attachments, 4 obsolete attachments)

(Assignee)

Description

4 years ago
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.
(Assignee)

Updated

4 years ago
Blocks: 812995
(Assignee)

Comment 1

4 years ago
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>.
(Assignee)

Comment 4

4 years ago
Created attachment 733263 [details] [diff] [review]
part.1 Drop text-decoration: blink; support

https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=1b296990ad76
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
(Assignee)

Comment 6

4 years ago
Created attachment 733326 [details] [diff] [review]
part.2 Drop <blink> support from editor
(Assignee)

Comment 7

4 years ago
Created attachment 733328 [details] [diff] [review]
part.3 Drop <blink> support from HTML parser

https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=c3daa815a8d8
(Assignee)

Comment 8

4 years ago
(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)

Updated

4 years ago
Assignee: nobody → masayuki
(Assignee)

Comment 9

4 years ago
FYI:

Following document uses <blink>:
http://mxr.mozilla.org/mozilla-central/source/tools/performance/layout/perf-doc.html?force=1#96
http://mxr.mozilla.org/mozilla-central/source/browser/app/profile/extensions/testpilot@labs.mozilla.com/content/debug.html?force=1#250

And following UI? uses |text-decoration: blink;|:
http://mxr.mozilla.org/mozilla-central/source/security/manager/pki/resources/content/createCertInfo.xul#26

I'll file bugs for them when this is fixed.
(Assignee)

Comment 10

4 years ago
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.
Attachment #733263 - Attachment is obsolete: true
Attachment #733713 - Flags: review?(dbaron)
(Assignee)

Comment 11

4 years ago
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+
(Assignee)

Comment 17

4 years ago
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)
(Assignee)

Comment 18

4 years ago
Comment on attachment 733326 [details] [diff] [review]
part.2 Drop <blink> support from editor

Also this.
Attachment #733326 - Flags: review?(mrbkap)
(Assignee)

Updated

4 years ago
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Assignee)

Comment 19

4 years ago
Created attachment 734176 [details] [diff] [review]
part.1 Drop blink effect implementation r=dbaron
Attachment #733713 - Attachment is obsolete: true

Updated

4 years ago
Keywords: dev-doc-needed
(Assignee)

Comment 20

4 years ago
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)
(Assignee)

Comment 23

4 years ago
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)
(Assignee)

Comment 24

4 years ago
(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)
(Assignee)

Comment 26

4 years ago
(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)
(Assignee)

Updated

4 years ago
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.
(Assignee)

Comment 28

4 years ago
(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.

Updated

4 years ago
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?

Updated

4 years ago
See Also: → bug 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.

Comment 42

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

Comment 45

4 years ago
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.
(Assignee)

Comment 46

4 years ago
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.
Flags: needinfo?(mrbkap)
Attachment #733328 - Flags: review?(mrbkap) → review+
(Assignee)

Updated

4 years ago
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

Comment 48

4 years ago
Created attachment 736112 [details] [diff] [review]
completely remove <blink> element
(Assignee)

Comment 49

4 years ago
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.
Attachment #733326 - Attachment is obsolete: true
Attachment #733326 - Flags: feedback?(daniel)
Attachment #736517 - Flags: review?(ehsan)
Attachment #736517 - Flags: review?(ayg)

Updated

4 years ago
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.
(Assignee)

Comment 54

4 years ago
(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.
(Assignee)

Comment 55

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ce50f99b680
https://hg.mozilla.org/integration/mozilla-inbound/rev/141e48a6df7c
https://hg.mozilla.org/integration/mozilla-inbound/rev/acfaca07e5c9
https://hg.mozilla.org/mozilla-central/rev/acfaca07e5c9
https://hg.mozilla.org/mozilla-central/rev/141e48a6df7c
https://hg.mozilla.org/mozilla-central/rev/6ce50f99b680
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
(Assignee)

Comment 57

4 years ago
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.
(Assignee)

Updated

4 years ago
Depends on: 861756
(Assignee)

Updated

4 years ago
Depends on: 861757
(Assignee)

Updated

4 years ago
Depends on: 861760
(Assignee)

Updated

4 years ago
Depends on: 861761
(Assignee)

Updated

4 years ago
Depends on: 861763

Updated

4 years ago
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

Comment 59

4 years ago
(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?
(Assignee)

Comment 60

4 years ago
(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 62

4 years ago
Filed https://www.w3.org/Bugs/Public/show_bug.cgi?id=21712 Thanks Masatoshi.
(Assignee)

Comment 63

4 years ago
Created attachment 738838 [details] [diff] [review]
part.4 followup fix (remove NOISY_BLINK)
Attachment #738838 - Flags: review?(dbaron)

Updated

4 years ago
relnote-firefox: --- → ?
Comment on attachment 738838 [details] [diff] [review]
part.4 followup fix (remove NOISY_BLINK)

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

Comment 65

4 years ago
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.
(Assignee)

Comment 66

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/1d6d1136ee15
https://hg.mozilla.org/mozilla-central/rev/1d6d1136ee15

Comment 68

4 years ago
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

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

Comment 71

4 years ago
I think you just broke all geocities pages.

Must keep a legacy browser around to experience them now.

Comment 72

4 years ago
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!
relnote-firefox: ? → 23+

Updated

4 years ago
Blocks: 873257

Updated

4 years ago
Blocks: 873258

Updated

4 years ago
Keywords: dev-doc-needed → doc-bug-filed

Updated

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

Comment 74

4 years ago
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

4 years ago
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

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

Updated

4 years ago
Depends on: 910664
You need to log in before you can comment on or make changes to this bug.