Last Comment Bug 485377 - Implement HTML5's <mark> tag
: Implement HTML5's <mark> tag
Status: RESOLVED FIXED
: dev-doc-complete, html5
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- enhancement with 1 vote (vote)
: mozilla2.0b1
Assigned To: :Ms2ger (⌚ UTC+1/+2)
:
Mentors:
http://mozilla.hoa-project.net/Test/M...
Depends on: 573377
Blocks: html5 568509 573357
  Show dependency treegraph
 
Reported: 2009-03-26 10:28 PDT by Ivan Enderlin
Modified: 2014-07-03 10:09 PDT (History)
16 users (show)
Ms2ger: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch 0.1: early draft (9.15 KB, patch)
2009-03-26 10:38 PDT, Ivan Enderlin
no flags Details | Diff | Splinter Review
Patch v2 (7.11 KB, patch)
2010-05-23 12:07 PDT, :Ms2ger (⌚ UTC+1/+2)
jonas: review+
Details | Diff | Splinter Review
Editor patch v1 (2.77 KB, patch)
2010-05-26 07:07 PDT, :Ms2ger (⌚ UTC+1/+2)
no flags Details | Diff | Splinter Review
Content patch v2 (merged to tip) (7.12 KB, patch)
2010-06-07 06:42 PDT, :Ms2ger (⌚ UTC+1/+2)
no flags Details | Diff | Splinter Review
Editor patch v2 (2.79 KB, patch)
2010-06-20 11:26 PDT, :Ms2ger (⌚ UTC+1/+2)
timeless: review+
Details | Diff | Splinter Review
Patch for checkin (9.30 KB, patch)
2010-06-21 05:32 PDT, :Ms2ger (⌚ UTC+1/+2)
no flags Details | Diff | Splinter Review

Description Ivan Enderlin 2009-03-26 10:28:50 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.3) Gecko/2008101315 Ubuntu/8.10 (intrepid) Firefox/3.0.3
Build Identifier: 

The HTML 5 draft recommendation describes the <mark> element as follow:
"The <mark> element represents a run of text in one document marked or
highlighted for reference purposes, due to its relevance in another context.
When used in a quotation or other block of text referred to from the prose, it
indicates a highlight that was not originally present but which has been added
to bring the reader's attention to a part of the text that might not have been
considered important by the original author when the block was originally
written, but which is now under previously unexpected scrutiny. When used in the
main prose of a document, it indicates a part of the document that has been
highlighted due to its likely relevance to the user's current activity.".

More examples at
<http://www.whatwg.org/specs/web-apps/current-work/#the-mark-element>

The draft does not precise a default style.

It is a semantic element, with generic HTML properties.

Reproducible: Always
Comment 1 Ivan Enderlin 2009-03-26 10:38:40 PDT
Created attachment 369531 [details] [diff] [review]
patch 0.1: early draft
Comment 2 Jonas Sicking (:sicking) No longer reading bugmail consistently 2009-04-06 17:52:08 PDT
You shouldn't need the new nsHTMLMarkElement.cpp, and indeed it doesn't seem like you're actually using it. The change to nsHTMLTagList.h ensures that we instantiate an nsHTMLSpanElement, which seems to be exactly what we want.
Comment 3 Henri Sivonen (:hsivonen) 2009-04-07 04:29:13 PDT
Does the <mark> element need un-<span>-like exposure to accessibility APIs?
Comment 4 Ivan Enderlin 2009-04-08 02:53:31 PDT
@Jonas: Yes, the nsHTMLTagList declaration asserts to use the nsHTMLSpanElement. Should it be suffisant to do that ? I have note that also the <abbr> element is recognized like a <span> (test: http://mozilla.hoa-project.net/Test/Abbr.html).

@Henri: I think no. I have looked for in the HTML 5 specification and I didn't find anything that is on the subject, but I may be wrong :-).
Comment 5 Henri Sivonen (:hsivonen) 2009-04-20 04:59:50 PDT
Perhaps the "Find Again" command should cycle through <mark> elements when the user hasn't entered a search string into the "Find" bar.
Comment 6 Ivan Enderlin 2009-04-20 05:12:22 PDT
Definitively. I am currently working on this feature.
Comment 7 Josh Tumath 2010-04-11 02:54:11 PDT
(In reply to comment #0)
> The draft does not precise a default style.

Actually, it does.

http://www.w3.org/TR/html5/the-xhtml-syntax.html#the-css-user-agent-style-sheet-and-presentational-hints
Comment 8 Henri Sivonen (:hsivonen) 2010-05-06 05:07:49 PDT
Not a parser bug, AFAICT.
Comment 9 d 2010-05-21 10:05:51 PDT
Would a patch for adding "mark { background: yellow; color: black; }" to html.css be better than not having any support at all, where the alternative is to implement everything (is there anything other than CSS and DOM changes to be done?) related to the "mark" element at a later date?

It could be noted that similar actions has already been taken for several other HTML5 elements, in bug 559284.
Comment 10 Jonas Sicking (:sicking) No longer reading bugmail consistently 2010-05-21 11:15:29 PDT
Why go for a half-assed implementation. There is a patch that is almost finished. Why not finish and land that instead.

Just remove the changes to nsGenericHTMLElement.h and remove the new nsHTMLMarkElement.* files. And add the html.css change in comment 9.

Then add a reftest checking that the default rendering is ok and that |document.createElement("mark") instanceof HTMLElement| is true
and
|document.createElement("mark") instanceof HTMLUnknownElement| is false
Comment 11 d 2010-05-21 12:34:04 PDT
Well, considering the patch is called "Patch 0.1: Early draft" and haven't been updated since early 2009, I assumed it was pretty "dead" and perhaps out of date. Still, the full implementation is of course what we want to do/have.
Comment 12 d 2010-05-21 12:36:34 PDT
Oh, and to avoid misunderstandings, this is not me taking on the task to update the patch. If it was as easy as just uploading a new version of the file, I probably could, but those patch/building instructions look too advanced for me, and I don't have the time to learn how to do it properly right now.
Comment 13 Ivan Enderlin 2010-05-21 12:43:37 PDT
The patch is not dead. I'm just waiting 2 days free to work on this patch…

If it is too easy as Jonas said, I can make it in June maybe (I hope).
Comment 14 :Ms2ger (⌚ UTC+1/+2) 2010-05-23 12:07:59 PDT
Created attachment 446980 [details] [diff] [review]
Patch v2

Updated to Jonas' comments. Ivan, I hope you don't mind.

Note that |document.createElement("mark") instanceof HTMLUnknownElement| isn't false. Apparently, all HTML elements implement HTMLUnknownElement.
Comment 15 Jonas Sicking (:sicking) No longer reading bugmail consistently 2010-05-26 02:54:42 PDT
Comment on attachment 446980 [details] [diff] [review]
Patch v2

Thanks!
Comment 16 :Ms2ger (⌚ UTC+1/+2) 2010-05-26 07:07:29 PDT
Created attachment 447511 [details] [diff] [review]
Editor patch v1

Turns out I needed to tell editor about it too... This change is covered by the following existing tests

docshell/test/navigation/test_bug386782.html
editor/composer/test/test_bug384147.html
editor/libeditor/html/tests/browserscope/test_browserscope.html
editor/libeditor/html/tests/test_bug478725.html
editor/libeditor/html/tests/test_bug487524.html
layout/generic/test/test_backspace_delete.xul
Comment 17 timeless 2010-05-26 10:59:50 PDT
i have a feeling this is going to conflict with bug 562008.
Comment 18 Ivan Enderlin 2010-05-27 01:59:25 PDT
@Ms2ger It's ok, I don't mind :-). I prefere to see this patch going further instead of resting in peace ;-).
Comment 19 Mounir Lamouri (:mounir) 2010-05-27 07:57:59 PDT
hmm, Ms2ger, your patch makes <mark> be an HTMLSpanElement. I don't think we want <mark> to be an HTMLSpanElement (even if that makes <mark> using HTMLElement interface) ?

IOW, document.createElement('mark').constructor should return HTMLElement, not HTMLSpanElement.
Comment 20 Ivan Enderlin 2010-05-27 08:12:50 PDT
@Mounir: That's what I thought at the begining, but in Gecko, the majority of semantic elements are reconized as HTMLSpanElement.
Comment 21 Mounir Lamouri (:mounir) 2010-05-27 08:17:52 PDT
(In reply to comment #20)
> @Mounir: That's what I thought at the begining, but in Gecko, the majority of
> semantic elements are reconized as HTMLSpanElement.

Indeed. Anyway, this is going to be fixed with bug 562008.
@Ms2ger: could you open a follow-up depending on bug 562008 and put me in CC ?
Comment 22 :Ms2ger (⌚ UTC+1/+2) 2010-06-07 06:42:18 PDT
Created attachment 449613 [details] [diff] [review]
Content patch v2 (merged to tip)

Merged to tip. r=jonas
Comment 23 :Ms2ger (⌚ UTC+1/+2) 2010-06-20 11:26:00 PDT
Created attachment 452587 [details] [diff] [review]
Editor patch v2

timeless, could you review this patch as well? Thanks!
Comment 24 :Ms2ger (⌚ UTC+1/+2) 2010-06-21 05:32:28 PDT
Created attachment 452701 [details] [diff] [review]
Patch for checkin

This patch contains attachment 449613 [details] [diff] [review] (r=jonas) and attachment 452587 [details] [diff] [review] (r=timeless), as well as attachment 447778 [details] [diff] [review] from bug 568509 (r=jonas).
Comment 25 Shawn Wilsher :sdwilsh 2010-06-23 10:13:40 PDT
Assuming this needs to be documented, so adding dev-doc-needed.

http://hg.mozilla.org/mozilla-central/rev/b4149e2c4056
Comment 26 Jean-Yves Perrier [:teoli] 2010-06-24 11:53:25 PDT
Documented as: https://developer.mozilla.org/en/HTML/Element/mark
(I'll update it once bug 568509 land).

As I cannot change the keyword dev-doc-needed to dev-doc-complete, if somebody could do it, it would be nice.

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