Closed Bug 1195801 Opened 8 years ago Closed 8 years ago

Add API function for fetching Microdata data to the Browser API

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
FxOS-S8 (02Oct)
Iteration:
42.3 - Aug 10
Tracking Status
firefox44 --- fixed

People

(Reporter: tedders1, Assigned: tedders1)

References

Details

(Keywords: dev-doc-complete)

Attachments

(2 files, 3 obsolete files)

      No description provided.
Assignee: nobody → tclancy
Iteration: --- → 42.3 - Aug 10
Attached patch bug-1195801-fix.patch (obsolete) — Splinter Review
Attachment #8649331 - Flags: review?(kchen)
This code fetches the Microdata and converts it into JSON, following the WHATWG recommendation.

https://html.spec.whatwg.org/multipage/microdata.html#json

This code also fetches Microformats data for two formats (hCard and hCalendar), which I consider to be legacy support. (The WHATWG spec for Microdata intends to eventually absorb hCard and hCalendar.)

https://html.spec.whatwg.org/multipage/microdata.html#vcard
https://html.spec.whatwg.org/multipage/microdata.html#vevent
Comment on attachment 8649331 [details] [diff] [review]
bug-1195801-fix.patch

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

Looks good. We need test cases, you could use dom/browser-element/mochitest/createNewTest.py to create templates

::: dom/browser-element/BrowserElementChildPreload.js
@@ +1587,5 @@
> +
> +  _processItem: function(node, alreadyProcessed) {
> +    if (alreadyProcessed.includes(node)) {
> +      return "ERROR";
> +    }

null check alreadyProcessed.

Do we really want to include "ERROR" in the result if bad things happen? I think we could use a debug message here.

@@ +1647,5 @@
> +
> +    sendAsyncMsg('got-structured-data', {
> +      id: data.json.id,
> +      successRv: resultString
> +    });

return result directly?
Attachment #8649331 - Flags: review?(kchen) → review+
Target Milestone: --- → FxOS-S5 (21Aug)
Target Milestone: FxOS-S5 (21Aug) → FxOS-S6 (04Sep)
Attached patch bug-1195801-fix.patch (obsolete) — Splinter Review
Attachment #8649331 - Attachment is obsolete: true
(In reply to Kan-Ru Chen [:kanru] from comment #3)
> 
> Do we really want to include "ERROR" in the result if bad things happen? I
> think we could use a debug message here.

The string "ERROR" is actually part of the WHATWG spec.

> return result directly?

The idea is to support the JSON format described in https://html.spec.whatwg.org/multipage/microdata.html#json, and JSON needs to be a string.
Attached patch bug-1195801-fix.patch (obsolete) — Splinter Review
Attachment #8661026 - Attachment is obsolete: true
This uses much the same test infrastructure as Bug 1169633.
Attachment #8662754 - Flags: review?(kchen)
Comment on attachment 8662753 [details] [diff] [review]
bug-1195801-fix.patch

There were some moderate changes to this file, so I'm flagging it for re-review. Sorry, Kanru.
Attachment #8662753 - Flags: review?(kchen)
Attachment #8662754 - Flags: review?(kchen) → review+
Attachment #8662753 - Flags: review?(kchen) → review+
Keywords: checkin-needed
this needs dom peer review for the change in dom/webidl/BrowserElement.webidl
Flags: needinfo?(tclancy)
Keywords: checkin-needed
Ted, please ask :bholly to review the WebIDL :)
Comment on attachment 8662753 [details] [diff] [review]
bug-1195801-fix.patch

Bobby, can you review the change to nsIBrowserElementAPI.idl?
Flags: needinfo?(tclancy)
Attachment #8662753 - Flags: review?(bobbyholley)
Comment on attachment 8662753 [details] [diff] [review]
bug-1195801-fix.patch

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

Please document that the actual value returned via the DOMRequest is a JSON string.
Attachment #8662753 - Flags: review?(bobbyholley) → review+
Attachment #8662753 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/39b57a8b21d5
https://hg.mozilla.org/mozilla-central/rev/78abc4497de5
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: FxOS-S6 (04Sep) → FxOS-S8 (02Oct)
When the docs are done, please be clear that this is not a standards complaint implementation of any semantic web technology.
Removed in bug 1273203.
You need to log in before you can comment on or make changes to this bug.