Closed
Bug 1262923
Opened 8 years ago
Closed 8 years ago
Create schema for the history API
Categories
(WebExtensions :: Untriaged, defect)
WebExtensions
Untriaged
Tracking
(firefox48 fixed)
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: bsilverberg, Assigned: bsilverberg)
References
Details
(Whiteboard: [history]triaged)
Attachments
(1 file)
Create a schema file for the history API. The Chrome docs for the API can be found at [1] and the Chrome schema can be found at [2]. [1] https://developer.chrome.com/extensions/history [2] https://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/extensions/api/history.json
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → bob.silverberg
Status: NEW → ASSIGNED
Iteration: --- → 48.3 - Apr 25
Updated•8 years ago
|
Whiteboard: [history]triaged
Assignee | ||
Comment 1•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/47731/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47731/
Attachment #8743385 -
Flags: review?(aswan)
Comment 2•8 years ago
|
||
Comment on attachment 8743385 [details] MozReview Request: Bug 1262923 - Create schema for the history API, r?aswan https://reviewboard.mozilla.org/r/47731/#review44877 ::: browser/components/extensions/ext-history.js:10 (Diff revision 1) > +extensions.registerSchemaAPI("history", "history", (extension, context) => { > + return { > + history: { > + search: function(query) { > + // FIXME: Implement history.search > + // This is just here so a test can be written that calls a history API method Lets not do this. If this lands as-is and somebody tries to use this method they are going to be very surprised. You can have an automated test check for the presence of chrome.history / browser.history but until we have live code for individual methods, I don't think there's any point testing for them. ::: browser/components/extensions/schemas/history.json:98 (Diff revision 1) > + } > + ], > + "functions": [ > + { > + "name": "search", > + "unsupported": false, apropos the above comment this should be true. ::: browser/components/extensions/test/mochitest/mochitest.ini:3 (Diff revision 1) > +[DEFAULT] > + > +[test_ext_history.html] This test works as a mochitest, but it seems likely that all the substantive tests will have to be browser_* tests? If that's the case, I would think it would be ideal to just have all the tests there rather than have this one lonely mochitest all by itself here...
Attachment #8743385 -
Flags: review?(aswan)
Assignee | ||
Comment 3•8 years ago
|
||
Comment on attachment 8743385 [details] MozReview Request: Bug 1262923 - Create schema for the history API, r?aswan Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47731/diff/1-2/
Attachment #8743385 -
Flags: review?(aswan)
Assignee | ||
Comment 4•8 years ago
|
||
https://reviewboard.mozilla.org/r/47731/#review44877 > Lets not do this. If this lands as-is and somebody tries to use this method they are going to be very surprised. > You can have an automated test check for the presence of chrome.history / browser.history but until we have live code for individual methods, I don't think there's any point testing for them. Yeah, I wasn't crazy about this either, and my intent was to land the implementation ASAP, but you're right, it's not a good idea. I'll just check for the existence of `browser.history` as you suggest. > This test works as a mochitest, but it seems likely that all the substantive tests will have to be browser_* tests? If that's the case, I would think it would be ideal to just have all the tests there rather than have this one lonely mochitest all by itself here... When I started working on `history.search()` I realized that I would need a browser test, so yeah, this makes sense.
Comment 5•8 years ago
|
||
Comment on attachment 8743385 [details] MozReview Request: Bug 1262923 - Create schema for the history API, r?aswan https://reviewboard.mozilla.org/r/47731/#review44941
Attachment #8743385 -
Flags: review?(aswan) → review+
Assignee | ||
Comment 6•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ad3010b9ffcc
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 8•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a07ea7fb572f
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•