Last Comment Bug 584285 - Meta tag list: HTML5-style charset is not displayed / meta tags in body are displayed
: Meta tag list: HTML5-style charset is not displayed / meta tags in body are ...
Status: NEW
:
Product: Firefox
Classification: Client Software
Component: Page Info Window (show other bugs)
: unspecified
: All All
: -- normal with 5 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
http://dev.w3.org/html5/html4-differe...
: 618763 664494 1013910 1075481 (view as bug list)
Depends on: 584326
Blocks:
  Show dependency treegraph
 
Reported: 2010-08-03 22:33 PDT by Kohei Yoshino [:kohei]
Modified: 2014-10-01 09:06 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (785 bytes, patch)
2010-08-04 04:10 PDT, Kohei Yoshino [:kohei]
no flags Details | Diff | Splinter Review
patch v2 (868 bytes, patch)
2010-12-09 22:50 PST, Kohei Yoshino [:kohei]
no flags Details | Diff | Splinter Review
patch v3 (1.05 KB, patch)
2011-09-18 07:53 PDT, Kohei Yoshino [:kohei]
MattN+bmo: feedback-
Details | Diff | Splinter Review

Description Kohei Yoshino [:kohei] 2010-08-03 22:33:27 PDT
HTML5 allows <meta charset="UTF-8"> instead of the traditional <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> format.

In the meta tag list, this new format is ignored and displayed as a blank line.

You can test this at http://mozilla.jp/

The expected result would be
Name: charset
Content: UTF-8
Comment 1 Philip Chee 2010-08-04 02:05:34 PDT
http://mxr.mozilla.org/mozilla-central/source/dom/interfaces/html/nsIDOMHTMLMetaElement.idl?force=1

Hmm nsIDOMHTMLMetaElement.idl doesn't have a HTML5 charset property. Which would make it easier for PageInfo.
Comment 2 :Ms2ger (⌚ UTC+1/+2) 2010-08-04 03:49:36 PDT
What's hard about getAttribute()?
Comment 3 Kohei Yoshino [:kohei] 2010-08-04 04:10:28 PDT
Created attachment 462743 [details] [diff] [review]
patch
Comment 4 Philip Chee 2010-08-04 04:41:44 PDT
Array.forEach(metaNodes, function(node) {
  if (node.hasAttribute("charset")
    gMetaView.addRow("charset", node.getAttribute("charset"));
  else
    gMetaView.addRow([metaNodes[i].name || metaNodes[i].httpEquiv, metaNodes[i].content]);
}
Comment 5 Kohei Yoshino [:kohei] 2010-12-09 18:48:30 PST
I just found that <meta property="*" content="*" /> tags were also not displayed properly. Such tags are used in the Open Graph protocol:
http://opengraphprotocol.org/

Will update the patch.
Comment 6 Kohei Yoshino [:kohei] 2010-12-09 19:03:12 PST
And we should also support the itemprop attributes.
http://www.whatwg.org/specs/web-apps/current-work/multipage/semantics.html#meta
Comment 7 Kohei Yoshino [:kohei] 2010-12-09 22:50:32 PST
Created attachment 496758 [details] [diff] [review]
patch v2
Comment 8 :Ms2ger (⌚ UTC+1/+2) 2010-12-10 00:51:30 PST
(In reply to comment #5)
> I just found that <meta property="*" content="*" /> tags were also not
> displayed properly. Such tags are used in the Open Graph protocol:
> http://opengraphprotocol.org/

I feel pretty strongly that we should not support those proprietary extensions.

(In reply to comment #6)
> And we should also support the itemprop attributes.
> http://www.whatwg.org/specs/web-apps/current-work/multipage/semantics.html#meta

Personally, I'm not sure we should do this until we support microdata (Bug 591467). Don't feel strongly about this one, though.
Comment 9 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2010-12-10 01:23:03 PST
(In reply to comment #5)
> I just found that <meta property="*" content="*" /> tags were also not
> displayed properly. Such tags are used in the Open Graph protocol:
> http://opengraphprotocol.org/

I think it doesn't make sense to display the property attribute an <meta> when RDFa allows it on any element. Furthermore, it doesn't make sense to display it without resolving the CURIE and without the context of the larger RDF graph. However, I think we should keep CURIEs out of Gecko and displaying a larger RDF graph in a user friendly way isn't (AFAIK) a solved UI problem in general.

I think we should keep CURIEs out of Gecko, because RDFa 1.0 CURIEs rely on markup features that parse differently in HTML and XHTML and, therefore, violate the HTML Design Principles. I think we should keep RDFa 1.1 CURIEs out of Gecko, because they have the same author confusion characteristics as XML Namespaces (see http://wiki.whatwg.org/wiki/Namespace_confusion ) and implementing them would either be bad for performance or require added complexity to Gecko's DOM implementation to make the prefix resolution fast.

Doing only what the patch does is a slippery slope towards doing more. People would cite is as precedent for *some* RDFa support and ask for more, which would bring in the above-mentioned CURIE badness and the RDF graph complexity issues.

As for the Open Graph Protocol specifically, I think we shouldn't add support for de facto vendor-specific markup (in this case de facto Facebook-specific, AFAICT) even if the markup has "Open" in its name.

(In reply to comment #6)
> And we should also support the itemprop attributes.
> http://www.whatwg.org/specs/web-apps/current-work/multipage/semantics.html#meta

If we add support for Microdata features, it would make sense to use the APIs from bug 591467 to do so. However, I think it doesn't make sense to single out the participation of the <meta> element in Microdata and show it detached from the larger Microdata data model just like it doesn't make sense to detach RDFa-participating <meta> elements from the larger RDF graph.

I suggest going back to attachment 462743 [details] [diff] [review]. r- from me on attachment 496758 [details] [diff] [review], though I realize I'm not the module owner.
Comment 10 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2010-12-12 19:56:21 PST
*** Bug 618763 has been marked as a duplicate of this bug. ***
Comment 11 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2010-12-13 15:47:51 PST
*** Bug 618763 has been marked as a duplicate of this bug. ***
Comment 12 Philip Chee 2011-06-15 11:37:08 PDT
*** Bug 664494 has been marked as a duplicate of this bug. ***
Comment 13 Kohei Yoshino [:kohei] 2011-09-18 07:52:49 PDT
OK, let's just forget the Open Graph Protocol.

I've found another issue: the <meta> elements in the <body> are listed.
e.g. http://schema.org/docs/gs.html#advanced_missing

> var metaNodes = gDocument.getElementsByTagName("meta");

querySelectorAll resolves the issue. Will update my patch soon.
Comment 14 Kohei Yoshino [:kohei] 2011-09-18 07:53:31 PDT
Created attachment 560790 [details] [diff] [review]
patch v3
Comment 15 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-09-19 03:36:22 PDT
Are we sure we don't want to show charset metas from outside the head? (They can take effect.)
Comment 16 Kohei Yoshino [:kohei] 2011-11-06 17:38:39 PST
I think this Meta Tag List feature in Firefox has intended to display metadata [1] specified in the <head>. So gDocument.querySelectorAll("head meta") would work.

[1] http://wiki.whatwg.org/wiki/MetaExtensions
Comment 17 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-11-06 22:24:48 PST
Is the patch considered ready for review at this point? I wonder if it's possible to write a unit tests for this with reasonable effort.
Comment 18 Kohei Yoshino [:kohei] 2013-11-11 19:39:38 PST
Still relevant, but I have no time to write tests. Resetting the assignee.
Comment 19 Kohei Yoshino [:kohei] 2014-05-21 07:44:36 PDT
*** Bug 1013910 has been marked as a duplicate of this bug. ***
Comment 20 Matthew N. [:MattN] 2014-05-30 23:23:19 PDT
Comment on attachment 560790 [details] [diff] [review]
patch v3

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

::: pageInfo.js
@@ +465,4 @@
>    document.getElementById("encodingtext").value = encoding;
>  
>    // get the meta tags
> +  var metaNodes = gDocument.querySelectorAll("head meta");

I think this should be done in a separate bug since it's orthogonal to the original issue.

@@ +482,5 @@
>  
> +    Array.forEach(metaNodes, function(node) {
> +      if (node.hasAttribute("charset")) {
> +        // get the HTML5-style short-hand charset
> +        gMetaView.addRow("charset", node.getAttribute("charset"));

This feels a bit weird since it's assigning a name of "charset" which could mislead people into thinking the page has <meta name="charset" content="…"> which is invalid. An alternative to this approach would be to simply omit <meta charset="…"> from the list since the resolved encoding is a few lines above the Meta tree. It also feels a bit weird to omit some <meta> elements but I think slightly less so.
Comment 21 Matthew N. [:MattN] 2014-10-01 09:06:06 PDT
*** Bug 1075481 has been marked as a duplicate of this bug. ***

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