Closed Bug 909633 Opened 11 years ago Closed 8 years ago

Remove HTML Microdata API

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: emk, Assigned: bzbarsky)

References

Details

(Keywords: addon-compat, dev-doc-complete, site-compat)

Attachments

(1 file)

Blink [1] and WebKit [2] removed the implementation. MacPort (a.k.a Safari) has never been enabled the feature. Trident has never been supported the feature [3]. Presto is dead. It was removed from the main HTML5 spec [4].
Perhaps the Web didn't need the feature. It even breaks some of the Web [5].

[1] https://chromium.googlesource.com/chromium/blink/+/88e7fb9f921306929788b79bdbfca85d727f589b
[2] http://trac.webkit.org/changeset/153772
[3] http://connect.microsoft.com/IE/feedback/details/793724/ie11-feature-request-support-for-microdata
[4] https://www.w3.org/Bugs/Public/show_bug.cgi?id=22783
[5] See bug 591467 blockers.
This is just to remove the JS API right? It would seem silly to remove the HTML microdata attributes themselves when [4] agrees to "continue HTML Microdata as a separate spec".
AFAIK we've supported only JS API from the start (I didn't see any layout/parser changes in the bug 591467 patch).
(In reply to Masatoshi Kimura [:emk] from comment #0)
> It was removed from the main HTML5 spec [4].
> [4] https://www.w3.org/Bugs/Public/show_bug.cgi?id=22783

What you point to is not a spec that informs our implementation.
(In reply to Masatoshi Kimura [:emk] from comment #0)
> Blink [1] and WebKit [2] removed the implementation.

This is relevant. The WebKit bug doesn't look particularly thoughtful when it comes to reasoning and the Blink commit message cites quality, maintainership and WebKit rather than any spec problem.

> It was removed from the main HTML5 spec

I think we shouldn't give this any weight. We shouldn't let the politics of how the W3C splices specs affect what we implement.
(In reply to Masatoshi Kimura [:emk] from comment #2)
> AFAIK we've supported only JS API from the start (I didn't see any
> layout/parser changes in the bug 591467 patch).
I could be mistaken but in the patch at
https://bugzilla.mozilla.org/attachment.cgi?id=629849
I saw lines like
+NS_IMPL_BOOL_ATTR(nsGenericHTMLElement, ItemScope, itemscope)
and
+  const nsAttrValue* attr = elem->GetParsedAttr(nsGkAtoms::itemtype);
(In reply to :Ms2ger from comment #3)
> (In reply to Masatoshi Kimura [:emk] from comment #0)
> > It was removed from the main HTML5 spec [4].
> > [4] https://www.w3.org/Bugs/Public/show_bug.cgi?id=22783
> 
> What you point to is not a spec that informs our implementation.

OK, filed a spec bug.
(In reply to Henri Sivonen (:hsivonen) from comment #4)
> This is relevant. The WebKit bug doesn't look particularly thoughtful when
> it comes to reasoning and the Blink commit message cites quality,
> maintainership and WebKit rather than any spec problem.

At least Chromium folks said they would not add any new API returning "live" node list.
Also, the API name conflicted with some legacy pages. It should have named as something like element.microdata.itemid or something.
(In reply to Masatoshi Kimura [:emk] from comment #6)
> (In reply to :Ms2ger from comment #3)
> > (In reply to Masatoshi Kimura [:emk] from comment #0)
> > > It was removed from the main HTML5 spec [4].
> > > [4] https://www.w3.org/Bugs/Public/show_bug.cgi?id=22783
> > 
> > What you point to is not a spec that informs our implementation.
> 
> OK, filed a spec bug.
>
> https://www.w3.org/Bugs/Public/show_bug.cgi?id=23080

I don't understand how 23080 is different from 22783.
Well, different specs. Bug 23080 is about the HTML spec we try to implement, Hixie as the editor.
Bug 22783 is about the W3C snapshot...as you very well know ;)
When I commented earlier they were both in the "HTML5 spec" component.
Blocks: 975653
Note that W3C has explicitly dropped microdata. http://lists.w3.org/Archives/Public/public-html-admin/2013Oct/0018.html We should move this along accordingly.
That's hardly an argument that carries any weight.
Please don't remove the Microdata API. We're using it for Bug 1195801.
> We're using it for Bug 1195801.

That's not a sufficient reason to not remove something that no one else is planning to implement.

I personally think we should remove it.  It adds a ton of complexity, adds a DOMStringList consumer, and if every single other UA has no plans to implement it, it's dead in spec terms....  Add to that the fact that it breaks some sites, and I see absolutely no reason to keep it.  I'm sorry we didn't move to remove it faster and now have internal consumers depending on it.  :(

Ted, how much of a problem would it be to use the normal DOM APIs for the functionality bug 1195801 added?  Because it seems to me like we should do that and then remove this cruft.
Flags: needinfo?(tclancy)
(In reply to Boris Zbarsky [:bz] from comment #14)
> Ted, how much of a problem would it be to use the normal DOM APIs for the
> functionality bug 1195801 added?  Because it seems to me like we should do
> that and then remove this cruft.

That can certainly be done. But, Bug 1195801 is an important part of the plans for Firefox OS 2.5, which is being finalized next month, so now would be a really bad time to start making unnecessary changes to how it works.

> and if every single other UA has no plans to implement it, it's dead in spec terms...

Well, maybe Firefox OS's great native support for Microdata (along the lines of: "This page has a RECIPE. Would you like to export that RECIPE to an app that manages RECIPES?") will inspire Apple and Google to implement similar support on their phones, which will be the thing that finally makes Microdata happen. Attempting to shape the web in this way is one of Mozilla's goals, so it's worth doing.

(And there really are recipe websites that mark up their recipes with Microdata. The two biggest ones both do it, bless them, though I'm not sure why.)

But I don't blame you for hating the API (it's pretty blergh), and I'm not even opposing its removal. I'm just saying that we might want to wait slightly longer before saying Microdata is dead.
Flags: needinfo?(tclancy)
> so now would be a really bad time to start making unnecessary changes to how it works

We've delayed on removing this for a while, so I'm not saying we need to do it this week.  But:

1)  Comment 13 indicates people knew this API was on the chopping block and went ahead and used it anyway.  Bygones and all, but I'm a bit confused by this decision.

2)  This API is gone from the spec.  At this point it's just a nonstandard feature we're shipping, and we should move to not doing so.

3)  As noted, us shipping this API breaks some websites.  It would be nice to get those fixed.

> which will be the thing that finally makes Microdata happen.

Microdata happening doesn't mean that we have to be shipping this API, yes?

> I'm just saying that we might want to wait slightly longer before saying Microdata is
> dead.

Sure.  And I'm trying to figure out a credible schedule for us being able to remove it, to unbreak websites.  It sounds like for the next month moving the FxOS stuff off it won't happen, but can it happen as a Q4 goal in general?
Flags: needinfo?(tclancy)
(In reply to Boris Zbarsky [:bz] from comment #16)
> 
> 1)  Comment 13 indicates people knew this API was on the chopping block and
> went ahead and used it anyway.  Bygones and all, but I'm a bit confused by
> this decision.

I did most of the work for Bug 1195801 before I discovered this ticket. (Yes, Bug 1195801 is dated Aug 18, but I did most of the work before creating the ticket. Sorry, I'm horrible that way.) I discovered this ticket when creating the ticket for Bug 1195801.

And even then, I didn't have the impression that we were actually going to get rid of the Microdata API. The person who created this ticket sounded like they were just proposing a suggestion. Until your comment two days ago (Comment 15), no one even made a strong statement in favour of removing the API. (Some comments were arguing for it, and some against, but no one seemed to care strongly either way.) And the ticket hadn't moved for more than a year (most of the comments were from 2013, and nothing newer than mid-2014), even though removing an API is pretty simple, so I honestly thought we were keeping the API.

And at the time I did my work, the API was still part of the WHATWG spec, which I thought was our guiding principle. It was only just removed from the spec 12 hours ago. (I checked the git log.) I only just noticed its absence now.

When I left Comment 13, I was doing so for posterity, in case this ticket was revisited in the future. I didn't know it was going to be an issue so soon. 

> 
> 2)  This API is gone from the spec.  At this point it's just a nonstandard
> feature we're shipping, and we should move to not doing so.

I agree. And the API is horrible. Let's get rid of it.

> Microdata happening doesn't mean that we have to be shipping this API, yes?

That's correct.

I was trying to warn that a resurgence of interest in Microdata might lead to demand for this API, but I was also under the mis-impression that the API was still a published standard.

> Sure.  And I'm trying to figure out a credible schedule for us being able to
> remove it, to unbreak websites.  It sounds like for the next month moving
> the FxOS stuff off it won't happen, but can it happen as a Q4 goal in
> general?

My understanding is that the phone should be done by mid-November. So yeah, late Q4 should be fine.
Flags: needinfo?(tclancy)
> And at the time I did my work, the API was still part of the WHATWG spec

With a longstanding spec issue to remove it, though that's not obvious from looking at the spec....  :(  I agree this was a sucky situation all around.

Anyway, as I said, that's all in the past now.  What we need is a bug filed blocking this one to migrate the work from bug 1195801 off this API, and some Q4 goals to do that.  Are you willing to do that?
Posted the site compatibility doc: https://www.fxsitecompat.com/en-US/docs/2015/microdata-api-will-be-removed-by-the-end-of-2015/

I myself have a Firefox app template engine powered by Microdata API, so rewriting it is my Q4 goal as well ;) Personally I'm very sad to see it go while Schema.org is getting popular these days.
Pulling out microdata use in bug 1037483 in the coming week.
Blocks: 802548
Ted, it's quite a bit past the end of Q4 and dom/browser-element/BrowserElementChildPreload.js still seems to be using the microdata API as far as I can tell.  Is that correct?

If so, what exactly is the plan here?  This is continuing to break sites, so I would _really_ like to remove this stuff...
Flags: needinfo?(ted.clancy)
Kan-Ru, see comment 21?  Do you know what the current stat of that file is, whether we need to keep it working, and if so what the plan to move it off this API is?
Flags: needinfo?(kchen)
I think Shane [:mixedpuppy] is working on related transitions. Also cc: [:overholt].
Note that I just realized if we're not going to fix BrowserElementChildPreload.js soonish, we can still unship this API on the web by marking it chromeonly.  But I would also really rather not cart around all this code...
(In reply to Boris Zbarsky [:bz] from comment #22)
> Kan-Ru, see comment 21?  Do you know what the current stat of that file is,
> whether we need to keep it working, and if so what the plan to move it off
> this API is?

I think this API could easily be implemented outside of BrowsnerElement so its safe to remove it. I'd like to know if any is actively working on it. Shane [:mixedpuppy] seems to be on leave until mid July.
Flags: needinfo?(kchen) → needinfo?(mixedpuppy)
I removed microdata from socialapi.  outside that I'm not working on anything related to microdata.
Flags: needinfo?(mixedpuppy)
> I think this API could easily be implemented outside of BrowsnerElement so its safe to remove it.

Aren't we running browserelement tests in automation?  Will they not start failing if I just remove the API?
Flags: needinfo?(kchen)
(In reply to Boris Zbarsky [:bz] from comment #27)
> > I think this API could easily be implemented outside of BrowsnerElement so its safe to remove it.
> 
> Aren't we running browserelement tests in automation?  Will they not start
> failing if I just remove the API?

I mean removing the API and its tests altogether.

Ben, are we still using this API for pin the web? If so, could we add it to the list of API that need re-implementation and set a timeframe to remove it from gecko?
Flags: needinfo?(kchen) → needinfo?(bfrancis)
Hi Kan-Ru,

As far as I know, the getStructuredData() method [0] of the Browser API is not currently used and has never been used by B2G.

We do use the metachange [1] event in B2G which is used to detect HTML <meta> tags, including those using RDFa markup for Open Graph metadata (it doesn't support microdata). This is used to generate a rich card representation of web pages similar to the experimental Activity Stream feature in Firefox and is still very much in active use.

(Note that once the pine branch merges back into mozilla-central, hopefully soon, the Browser API in general will be chrome-only which will limit the exposure of these features even further).

Hope this helps, and thanks for asking.

0. https://developer.mozilla.org/en-US/docs/Web/API/HTMLIFrameElement/getStructuredData
1. https://developer.mozilla.org/en-US/docs/Web/Events/mozbrowsermetachange
Flags: needinfo?(bfrancis)
Ah, so "this api" in comment 25 refers to the "getStructuredData" API on BrowserElement?  I thought it meant the entire BrowserElement API.  Filed bug 1273203 to remove getStructuredData.
Depends on: 1273203
Flags: needinfo?(ted.clancy)
Ben, could you review this in general?

James, can you please review the change to testing/web-platform/tests/tools/scripts/id2path.json?
Attachment #8753159 - Flags: review?(james)
Attachment #8753159 - Flags: review?(bkelly)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
(In reply to Boris Zbarsky [:bz] from comment #30)
> Ah, so "this api" in comment 25 refers to the "getStructuredData" API on
> BrowserElement?  I thought it meant the entire BrowserElement API.  Filed
> bug 1273203 to remove getStructuredData.

Yes, I thought this is the getStructuredData bug. Sorry for the confusion.
Comment on attachment 8753159 [details] [diff] [review]
Remove the HTML Microdata API, since no one else ended up implementing it and now it's been removed from the spec

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

wpt changes seem reasonable to me.
Attachment #8753159 - Flags: review?(james) → review+
Comment on attachment 8753159 [details] [diff] [review]
Remove the HTML Microdata API, since no one else ended up implementing it and now it's been removed from the spec

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

Looks good.  Did my best, but its a big patch so I could have missed something.  r=me with items addressed.

::: dom/html/nsGenericHTMLElement.cpp
@@ +568,5 @@
>  {
>    if (IsInUncomposedDoc()) {
>      UnregAccessKey();
>    }
>    

nit: Can you fix this trailing whitespace while you are here?

::: testing/web-platform/meta/html/dom/interfaces.html.ini
@@ +561,5 @@
>  
>    [HTMLElement interface: document.createElement("noscript") must inherit property "translate" with the proper type (2)]
>      expected: FAIL
>  
> +  [HTMLElement interface: document.createElement("noscript") must inherit property "dropzone" with the proper type (13)]

Is this just because the number changes since we have fewer properties now?

::: testing/web-platform/meta/html/infrastructure/urls/resolving-urls/query-encoding/windows-1251.html.ini
@@ -75,5 @@
>    [Getting <script>.src]
>      expected: FAIL
>  
> -  [Getting <div>.itemid]
> -    expected: FAIL

Do we need these lines any more?

  [microdata values <audio src>]
    expected: FAIL

  [microdata values <embed src>]
    expected: FAIL

  [microdata values <iframe src>]
    expected: FAIL

  [microdata values <img src>]
    expected: FAIL

  [microdata values <source src>]
    expected: FAIL

  [microdata values <track src>]
    expected: FAIL

  [microdata values <video src>]
    expected: FAIL

  [microdata values <a href>]
    expected: FAIL

  [microdata values <area href>]
    expected: FAIL

  [microdata values <link href>]
    expected: FAIL

  [microdata values <object data>]
    expected: FAIL
Attachment #8753159 - Flags: review?(bkelly) → review+
> nit: Can you fix this trailing whitespace while you are here?

Done.

> Is this just because the number changes since we have fewer properties now?

Yep.  The test sticks a running property count in its assertion messages (icky behavior of it), so any time you remove or add a property the messages for everything after that property change.

> Do we need these lines any more?

Good catch!  I expect we do not, but will verify locally and remove if so.

Thank you for slogging through that!
Keywords: addon-compat
https://hg.mozilla.org/mozilla-central/rev/93d5271f0332
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
(In reply to Jean-Yves Perrier [:teoli] from comment #39)
> I have added a clear warning and updated compat tables for:
> https://developer.mozilla.org/en-US/docs/Web/HTML/Microdata

Microdata itself is not dead. It's widely used mainly for Schema.org. Just the DOM API wasn't successful, unfortunately.
I have added a clear warning and updated compat tables for:
https://developer.mozilla.org/en-US/docs/Web/HTML/Microdata
and
https://developer.mozilla.org/en-US/docs/Web/API/Microdata_DOM_API

We will be able to move them to Archive in the future.

I've removed all mentions of the Microdata-related global attributes there: https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes

and of course added a mention in:
https://developer.mozilla.org/en-US/Firefox/Releases/49#HTML
Should we relnote this?
Flags: needinfo?(jypenator)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: