1005488 - deprecate content/content ?

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P2)

Description

[Tomislav Jovanovic :zombie]

i think those were meant to be deprecated along with Widget, the only module that still uses them..

Comment 1

[Tomislav Jovanovic :zombie]

ok, Symbiont was moved to deprecated/symbiont, but the docs were not updated:

https://developer.mozilla.org/en-US/Add-ons/SDK/Low-Level_APIs/content_symbiont

Comment 2

[Jordan Santell [:jsantell] [@jsantell]]

Looks like sdk/content/content is just a meta file, and only being used by Widget. Let's get rid of this.

Comment 3

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: GH PR 1482

Comment 4

[Jordan Santell [:jsantell] [@jsantell]]

And looks like content/content and content/symbiont should just be removed from docs all together

Comment 5

[Tomislav Jovanovic :zombie]

Comment on attachment 8417538 [details] [review]
GH PR 1482

i don't think we can do that straight away. i have done a MXR search for the use of this module, and there seem to be lots of addons that do.

so deprecate?

Comment 6

[Tomislav Jovanovic :zombie]

(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #4)
> And looks like content/content and content/symbiont should just be removed
> from docs all together

but this is probably a safe bet..

Comment 7

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8417538 [details] [review]
GH PR 1482

Just wrapped in deprecation functions for now.. can remove in a few months

Comment 8

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/84334a788980572071bd5aa9c9f854cf6d2cc972
Bug 1005488 - Deprecate sdk/content/content

https://github.com/mozilla/addon-sdk/commit/b7e143433f0dd2adfec605f70fd1f08e579c71e8
Merge pull request #1482 from jsantell/1005488-remove-content-content

Fix Bug 1005488 - Deprecate sdk/content/content, r=@zombie

Comment 9

[Christian Ascheberg]

Did this cause bug 1012467? And if so, was it intended?

Comment 10

[Tomislav Jovanovic :zombie]

this was definitively not intended. 

Symbiont is not used as a function, but as an object instead, so this change breaks it (also probably true for Loader).

i though it strange when i couldn't find any use of the deprecateFunction() for our "constructor" objects/functions, but i still missed this.

Jordan, i think a simple __proto__ surgery would work here, or maybe better to define a deprecateObject() ?

and btw, does this mean a test might be warranted?

Comment 11

[Jordan Santell [:jsantell] [@jsantell]]

Definitely a test should be warranted. Symbiont is a weird beast, not sure if just adding the deprecateFunction wrapper around it's proto objects is sufficient, but whatever is the easiest thing to do to alert a deprecation notice, and things still work. Symbiont is brutal.

Comment 12

[Tomislav Jovanovic :zombie]

Attachment: Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1500

didn't want to add a separate test file for the module we are deprecating, so i used test-content-symbiont.

Comment 13

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8430228 [details] [review]
Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1500

Quick nit on testing that they are also the expected objects

Comment 14

[Jordan Santell [:jsantell] [@jsantell]]

Disregard the nit, saw you're also requiring the objects directly from outside of sdk/content/content. Looks good!

Comment 15

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/e481b06c8ace69d5c901a652749cba11401b038c
bug 1005488 - deprecate content/content

https://github.com/mozilla/addon-sdk/commit/df7224da5a3d3c39b6799d288dd2c8cce59d693b
Merge pull request #1500 from zombie/1005488-deprecate-content

bug 1005488 - deprecate content/content, r=@jsantell

Comment 16

[Tomislav Jovanovic :zombie]

Attachment: Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1503

our tests never clean prefs after themselves :(

as a result, success sometimes depends on the execution order, which can differ across platforms..

Comment 17

[Christian Ascheberg]

Is the updated patch going to be uplifted to Firefox before the next release date?

Comment 18

[Tomislav Jovanovic :zombie]

Christian, we will definitely make sure this doesn't break things, so if we miss the standard trains, we can request Aurora uplift.

Comment 19

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8432096 [details] [review]
Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1503

Ah, looks good, I should've caught this earlier like our other deprecated tests

Comment 20

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/41818f69a55a530a5ee3e43bb1dcf224b8b1a4ce
bug 1005488 - deprecate content/content

our tests never clean prefs after themselves

https://github.com/mozilla/addon-sdk/commit/6daac81f08718cb539e74af0c8c7ee95b7ba49a2
Merge pull request #1503 from zombie/1005488-deprecate-content

bug 1005488 - deprecate content/content, set test pref, r=@jsantell

Comment 21

[Jordan Santell [:jsantell] [@jsantell]]

Resolving