Closed Bug 962626 Opened 6 years ago Closed 6 years ago

Browser API: Access value of <meta name="application-name" content=""> element

Categories

(Core :: DOM: Core & HTML, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: benfrancis, Assigned: baku)

References

Details

(Keywords: dev-doc-complete)

Attachments

(1 file, 3 obsolete files)

The W3C's HTML spec specifies a meta tag which has name="application-name" and can be used to specify the name of a web application, as distinct from the title of the current page in that application, which is defined by the title tag. http://dev.w3.org/html5/markup/meta.name.html

This meta tag is used fairly commonly on the web and we'd like to be able to access it in Gaia to display as a short title for web pages and to use when bookmarking a URL.

We currently have a mozbrowsertitle change event to detect when the <title> element of the current document has changed. Perhaps this should be an extension of that, or a separate mozbrowserapptitlechange event or similar?
Note that the manifest spec says that the manifest name should be used instead of this attribute when available <http://w3c.github.io/manifest/#name-member>.
Attached patch meta.patch (obsolete) — Splinter Review
This is a proposal.
This patch implements a new event called 'metachange' that is triggered when a new Meta element name="something" is added to the DOM.

This patch emits this events only for 'application-name' but it's easy to add other names as such as 'author', 'description', etc if needed.

It has mochitest as well.
Attachment #8364256 - Flags: review?(ehsan)
Attachment #8364256 - Flags: feedback?(bfrancis)
Comment on attachment 8364256 [details] [diff] [review]
meta.patch

That sounds great!
Attachment #8364256 - Flags: feedback?(bfrancis) → feedback+
Ehsan,

I would love to implement the manifest spec too if we think that's feasible.

The use case described in the manifest spec is when navigator.requestBookmark() is called by a web page itself. How do you imagine we would handle the case where the user bookmarks a page using UI in the user agent (or probably the Gaia system app in this case)? Currently we just use the URL contained in the latest mozbrowserlocationchange event when creating a bookmark, how would we access the manifest data from (privileged) content?

There are two potential use cases I can think of for Gaia...

1) Using the name property from the manifest in the Rocketbar title (which is displayed in the status bar at the top of the screen) before and/or after bookmarking.
2) Using the name property (and other data like icon URLs) when bookmarking the current page to the homescreen.

For 2, perhaps the system app could call a requestBookmark() method on a mozbrowser iframe and get the manifest data in a callback? Then fall back to the title from mozbrowsertitlechange if no manifest was provided.

For 1, I'm not sure how you could access the name from the manifest without requesting a bookmark?

For the use case in the spec of the page calling navigator.requestBookmark() itself, would we need some kind of mozbrowserrequestbookmark event?
I'd also like to be involved with the implementation, but need someone to mentor me (as I'm new to the project and I'm not sure where/how to contribute). I've got a rough reference implementation in JS of some aspects of the manifest that I'm doing in parallel as I'm writing the spec to verify it works and to produce a test suite. Just let me know how I can help.
On second thoughts, for both use cases 1 and 2 we could just use a mozbrowsermanifestchanged event which contains the URL of the manifest, like we do for icons. We can then either fetch and cache the manifest immediately to get the application name for use in a title bar, or store the URL for later in case the user tries to bookmark the page either from the user agent, or with a mozbrowserrequestbookmark event.

If that sounds reasonable, should I file separate bugs for those events?
Marcos,

In terms of an implementation of the manifest spec for Firefox OS, I *think* all we'd need to do is add the two events I described to the Browser API, then the rest could be implemented in Gaia's system app.

Perhaps Andrea or Kanru could help you with the Gecko part, I could help with the Gaia part.
(In reply to Ben Francis [:benfrancis] from comment #4)
> Ehsan,
> 
> I would love to implement the manifest spec too if we think that's feasible.
> 
> The use case described in the manifest spec is when
> navigator.requestBookmark() is called by a web page itself. How do you
> imagine we would handle the case where the user bookmarks a page using UI in
> the user agent (or probably the Gaia system app in this case)? Currently we
> just use the URL contained in the latest mozbrowserlocationchange event when
> creating a bookmark, how would we access the manifest data from (privileged)
> content?

To be clear, I wasn't suggesting anything about the process of bookmarking a page.  The manifest spec defines the name of the web application here: <http://w3c.github.io/manifest/#name-member>.  I guess the confusion is caused by the fact that step 2.2 in that section talks about what happens if requestBookmark was called by the page itself.  Marcos, do you think that we should change the prose in that section to only talk about that if the steps are run as a result of a call to requestBookmark()?

What I was suggesting in comment 1 was that if we decide to use the application-name meta tag, perhaps we should also consider letting the manifest name, if one is available, supersede that.

> There are two potential use cases I can think of for Gaia...
> 
> 1) Using the name property from the manifest in the Rocketbar title (which
> is displayed in the status bar at the top of the screen) before and/or after
> bookmarking.
> 2) Using the name property (and other data like icon URLs) when bookmarking
> the current page to the homescreen.
> 
> For 2, perhaps the system app could call a requestBookmark() method on a
> mozbrowser iframe and get the manifest data in a callback? Then fall back to
> the title from mozbrowsertitlechange if no manifest was provided.

Why would we need to call requestBookmark() there?  We can just invoke the steps for processing the manifest directly, right?

> For 1, I'm not sure how you could access the name from the manifest without
> requesting a bookmark?

I don't think there is anything which restricts accessing the manifest to a call to requestBookmark().

> For the use case in the spec of the page calling navigator.requestBookmark()
> itself, would we need some kind of mozbrowserrequestbookmark event?

Sure, but that would be a discussion which we should have when implementing navigator.requestBookmark().

(In reply to Ben Francis [:benfrancis] from comment #6)
> On second thoughts, for both use cases 1 and 2 we could just use a
> mozbrowsermanifestchanged event which contains the URL of the manifest, like
> we do for icons. We can then either fetch and cache the manifest immediately
> to get the application name for use in a title bar, or store the URL for
> later in case the user tries to bookmark the page either from the user
> agent, or with a mozbrowserrequestbookmark event.
> 
> If that sounds reasonable, should I file separate bugs for those events?

I don't think that it makes sense to have a manifestchanged event for both the title and the icon, because each one have their own fallback paths which might be skipped by unifying these events.  For example, if the application specifies the manifest that has an icon but no name, and then injects an application-name meta tag dynamically into the document, the first mozbrowsermanifestchanged event will fire, and we would not detect a name property in the manifest, neither would we detect an application-name meta tag, and later on when the meta tag is inserted into the document there will be no further mozbrowsermanifestchanged events to dispatch, so we would lose the new information about the name of the application from the meta tag.
Flags: needinfo?(mcaceres)
(In reply to Marcos Caceres [:marcosc] from comment #5)
> I'd also like to be involved with the implementation, but need someone to
> mentor me (as I'm new to the project and I'm not sure where/how to
> contribute). I've got a rough reference implementation in JS of some aspects
> of the manifest that I'm doing in parallel as I'm writing the spec to verify
> it works and to produce a test suite. Just let me know how I can help.

I'd also be happy to help you on the Gecko side.  See these instructions <https://developer.mozilla.org/en/docs/Simple_Firefox_build> for building Firefox locally, and <https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/Building> for building Firefox OS locally.  Once you have set up a local build, we can take things further (off this bug of course.)
Comment on attachment 8364256 [details] [diff] [review]
meta.patch

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

So this patch looks mostly fine, but before I spend more time to review it, Marcos, do you think we should look at the manifest name here or is it OK if we ignore that for now?  This patch currently only looks at the meta application-name tag.
Attachment #8364256 - Flags: feedback?(mcaceres)
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #10)
> So this patch looks mostly fine, but before I spend more time to review it,
> Marcos, do you think we should look at the manifest name here or is it OK if
> we ignore that for now?  

Ignore manifest name for now. It will be a few months yet before the manifest stuff is stable.
Flags: needinfo?(mcaceres)
Just at note on the patch: it seems to be missing support for language. See application-name selection algorithm:

http://www.whatwg.org/specs/web-apps/current-work/#meta-application-name
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #8)
> To be clear, I wasn't suggesting anything about the process of bookmarking a
> page.  The manifest spec defines the name of the web application here:
> <http://w3c.github.io/manifest/#name-member>.  I guess the confusion is
> caused by the fact that step 2.2 in that section talks about what happens if
> requestBookmark was called by the page itself.  Marcos, do you think that we
> should change the prose in that section to only talk about that if the steps
> are run as a result of a call to requestBookmark()?

Please see below... 

> What I was suggesting in comment 1 was that if we decide to use the
> application-name meta tag, perhaps we should also consider letting the
> manifest name, if one is available, supersede that.

That's exactly what the spec currently tries to specify (maybe it's not clear enough or I need an example?). 

`requestBookmark()` and the user initiating the request to bookmark through the browser's UI are treated the same. The UA is supposed to first check for a manifest (and d/l it if it hasn't already), then check if "name" is available, and if not available fallback to HTML "application-name".
Discussed with Andrea about language. It can be determined by looking at lang of the element or walking up the tree to find the first ancestor with a `lang` attribute.
Attached patch meta.patch (obsolete) — Splinter Review
Lang is taken from the element or the parents.
Attachment #8364256 - Attachment is obsolete: true
Attachment #8364256 - Flags: review?(ehsan)
Attachment #8364256 - Flags: feedback?(mcaceres)
Attachment #8365169 - Flags: review?(ehsan)
Attachment #8365169 - Flags: feedback?(mcaceres)
Comment on attachment 8365169 [details] [diff] [review]
meta.patch

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

Looks ok to me.
Attachment #8365169 - Flags: feedback?(mcaceres) → feedback+
(In reply to comment #13)
> > What I was suggesting in comment 1 was that if we decide to use the
> > application-name meta tag, perhaps we should also consider letting the
> > manifest name, if one is available, supersede that.
> 
> That's exactly what the spec currently tries to specify (maybe it's not clear
> enough or I need an example?). 
> 
> `requestBookmark()` and the user initiating the request to bookmark through the
> browser's UI are treated the same. The UA is supposed to first check for a
> manifest (and d/l it if it hasn't already), then check if "name" is available,
> and if not available fallback to HTML "application-name".

I think both of these points could use some clarification in the spec.
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #17)
> I think both of these points could use some clarification in the spec.

Agree. Will ping you once I finish defining the fallback model for a review. Hope to have all that done by end of next week. If you have any other spec feedback, please feel free to file bugs here:

https://github.com/w3c/manifest/issues
Comment on attachment 8365169 [details] [diff] [review]
meta.patch

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

::: dom/browser-element/BrowserElementChildPreload.js
@@ +505,5 @@
>        }
>      }, this);
>    },
>  
> +  // Processes the "name" field in <meta> tags and forward to specific handlers.

Please remove the comment about the specific handlers.

@@ +519,5 @@
> +    if (!e.target.name) {
> +      return;
> +    }
> +
> +    let handlers = { 'application-name' : null };

You're not using this for anything.

@@ +522,5 @@
> +
> +    let handlers = { 'application-name' : null };
> +
> +    debug('Got metaAdded: (' + e.target.name + ') ' + e.target.content);
> +    if (e.target.name in handlers) {

Just compare the name to 'application-name'.

@@ +535,5 @@
> +          break;
> +        }
> +      }
> +
> +      sendAsyncMsg('metachange', meta);

This algorithm still doesn't select the first meta element in tree order, as specified in step 5 of <http://www.whatwg.org/specs/web-apps/current-work/#meta-application-name>, so it doesn't conform to the spec.
Attachment #8365169 - Flags: review?(ehsan) → review-
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #19)
> This algorithm still doesn't select the first meta element in tree order, as
> specified in step 5 of
> <http://www.whatwg.org/specs/web-apps/current-work/#meta-application-name>,
> so it doesn't conform to the spec.

So I talked about this with Andrea on IRC, and we both agree that this is indeed an edge case.  For the majority of cases where there is only one meta application-name in the document, Andrea's patch will pick that up correctly.  The only case where this issue will result in us picking the wrong meta tag is when a second-best meta application-name tag is injected into the DOM tree *after* another one that would be picked otherwise.

Fabrice, Marcos, do you guys think we should block this patch on fixing that problem?  Or can it land with its current behavior?
Flags: needinfo?(mcaceres)
Flags: needinfo?(fabrice)
I think it's fine with the current behavior.
Flags: needinfo?(mcaceres)
Attached patch meta.patch (obsolete) — Splinter Review
Attachment #8365169 - Attachment is obsolete: true
Attachment #8365387 - Flags: review?(ehsan)
That's fine for me too.
Flags: needinfo?(fabrice)
Comment on attachment 8365387 [details] [diff] [review]
meta.patch

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

::: dom/browser-element/BrowserElementChildPreload.js
@@ +527,5 @@
> +      for (let elm = e.target;
> +           elm && elm.nodeType == e.target.ELEMENT_NODE;
> +           elm = elm.parentNode) {
> +        if (elm.getAttribute('lang')) {
> +          meta.lang = elm.getAttribute('lang');

You need to handle the xml:lang attribute here as well: <http://www.whatwg.org/specs/web-apps/current-work/#language>

Also, this code doesn't properly handle falling back to the document's content language.  In C++ you would call nsIDocment::GetContentLanguage() but I'm not sure if that's accessible in JS.  Perhaps you should expose that through a ChromeOnly getter or some such.

::: dom/browser-element/mochitest/browserElement_Metachange.js
@@ +12,5 @@
> +  return 'data:text/html,<html><head>' + meta + '<body></body></html>';
> +}
> +
> +function createHtmlWithLang(meta, lang) {
> +  return 'data:text/html,<html lang="' + lang + '"><head>' + meta + '<body></body></html>';

Please add tests for the xml:lang attrib as well.

Also, please add tests for documents which use a Content-Language header.  You can use a sjs script for that.

@@ +126,5 @@
> +  });
> +
> +
> +  iframe1.src = createHtml(createMeta('application-name', 'foobar'));
> +  // We should not recieve icon change events for either of the below iframes

Nit: s/icon/meta/
Attachment #8365387 - Flags: review?(ehsan) → review-
Attached patch meta.patchSplinter Review
Attachment #8365387 - Attachment is obsolete: true
Attachment #8367682 - Flags: review?(ehsan)
Comment on attachment 8367682 [details] [diff] [review]
meta.patch

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

::: dom/browser-element/BrowserElementChildPreload.js
@@ +529,5 @@
> +
> +      for (elm = e.target;
> +           !lang && elm && elm.nodeType == e.target.ELEMENT_NODE;
> +           elm = elm.parentNode) {
> +        if (elm.getAttribute('lang')) {

Nit: hasAttribute.

@@ +534,5 @@
> +          lang = elm.getAttribute('lang');
> +          continue;
> +        }
> +
> +        if (elm.getAttributeNS('http://www.w3.org/XML/1998/namespace', 'lang')) {

Nit: hasAttributeNS.

@@ +542,5 @@
> +
> +        if (elm.getAttribute('xml:lang')) {
> +          lang = elm.getAttribute('xml:lang');
> +          continue;
> +        }

Hmm, the spec says:

"The attribute in no namespace with no prefix and with the literal localname "xml:lang" has no effect on language processing."

http://www.whatwg.org/specs/web-apps/current-work/#attr-xml-lang

I think you should drop this case.

@@ +545,5 @@
> +          continue;
> +        }
> +      }
> +
> +      // No lang as been detected.

Nit: has
Attachment #8367682 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/mozilla-central/rev/d696c67e2489
Assignee: nobody → amarchesini
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Keywords: dev-doc-needed
\o/ Thanks guys.
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.