Closed
Bug 572977
Opened 14 years ago
Closed 14 years ago
drop requirement that tabs "url" parameters be URL objects
Categories
(Add-on SDK Graveyard :: General, defect)
Add-on SDK Graveyard
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: myk, Assigned: myk)
References
Details
Attachments
(1 file)
12.56 KB,
patch
|
dietrich
:
review+
|
Details | Diff | Splinter Review |
Tabs is currently the only API that requires "url" arguments to be URL objects, which makes its API anomalous. But it isn't the kind of API about which there is the security concern that prompted the requirement, since it doesn't accept HTML content, so such a requirement is also unwarranted in its case. Meanwhile, the requirement significantly complicates the simple case of loading a URL in a tab, which currently looks like this: require("tabs").open(require("url").URL("http://www.example.com/")); when, if the API accepted strings, it could look like this much simpler and more readable call instead: require("tabs").open("http://www.example.com/"); This is totally my bad for not paying enough attention during the review phase and thinking through the ramifications of my earlier decision to standardize on URL objects across the API set. And it's very late in the 0.5 cycle to be taking changes like this. Nevertheless, I think it's worth taking a late-breaking fix to drop the requirement that these URL arguments be URL objects. Then, as part of the 0.6 cycle, I'll take another look at the plan to standardize on URL objects across the API set and figure out what else (if anything) makes sense to do here.
Assignee | ||
Comment 1•14 years ago
|
||
Hmm, I just noticed that the tabs docs actually say "open" takes a string rather than a URL object, and they give several examples of that usage, which makes this more a blocker (whether we fix it by dropping the requirement or by fixing the docs) than a late-breaking change, since the docs are very out-of-sync with the code. Dietrich: do you agree?
Assignee | ||
Comment 2•14 years ago
|
||
Updated•14 years ago
|
Version: unspecified → Trunk
Comment 3•14 years ago
|
||
Comment on attachment 452198 [details] [diff] [review] patch v1: drops requirement yeah, strings are *way* nicer on the eyes. for a param that can only be a URL, strings should be fine. r=me.
Attachment #452198 -
Flags: review?(dietrich) → review+
Assignee | ||
Comment 4•14 years ago
|
||
Dietrich: are you cool with us landing this for 0.5 and respinning for it?
Comment 5•14 years ago
|
||
Agreed. Aside from being hard on the eyes, it also complicates things for newcomers to the platform: in the tutorial in bug 566261, when we want to simply open a tab to a website, adding require("url").URL() requires us to explain what this mysterious URL() function is--and begs the question of why a simple string can't be passed in (which as Myk mentioned in comment 1 we don't actually have a good answer to).
Comment 6•14 years ago
|
||
yes, land and respin! Yeah, i misunderstood when we talked about this way-back-when in MV. i thought you guys wanted all URLs everywhere in the SDK to be URL objects, so was embarking on that mission. but it's a flawed mission.
Assignee | ||
Comment 7•14 years ago
|
||
Fixed by changeset https://hg.mozilla.org/labs/jetpack-sdk/rev/3236e68e6f2e.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 8•14 years ago
|
||
The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product. To filter bugmail related to this change, filter on the word "looptid".
Component: Jetpack SDK → General
Product: Mozilla Labs → Add-on SDK
QA Contact: jetpack-sdk → general
Version: Trunk → unspecified
You need to log in
before you can comment on or make changes to this bug.
Description
•