Remove social.initialize-response message

RESOLVED FIXED in Firefox 17

Status

()

Firefox
SocialAPI
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: markh, Assigned: markh)

Tracking

unspecified
Firefox 18
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox17 fixed)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
WorkerAPI.jsm has support for a 'social.initialize-response' message, which sets 'this.initialized' to true.  However, this message and attribute are only used by the test suite and can almost certainly be removed without losing any test coverage or functionality.  The way it is setup implies it is a "real" message which must be sent by providers (and no current providers do that).  Thus it should be removed (or worst-case, replaced with something which makes it obvious it is purely for internal tests and is not expected to be implemented by real providers).
(Assignee)

Comment 1

5 years ago
Created attachment 658339 [details] [diff] [review]
Remove the message

Turns out to be relatively simple to remove.

Try run at https://tbpl.mozilla.org/?tree=Try&rev=28fc9d15e3b8
Attachment #658339 - Flags: review?(jaws)
Attachment #658339 - Flags: feedback?(mixedpuppy)
I expected this to be implemented by real providers, which is why I added it. I didn't do a very good job of advertising that expectation, though, which explains why no one does :)

Some social functionality depends on the livelihood of the worker (i.e. whether the worker has actually responded, rather than just whether we had attempted to create it and were queuing messages for it). I envisioned this as the mechanism for client code to detect that - for example, to only show the "share" button when we knew that the message it sends will be received.
(Assignee)

Comment 3

5 years ago
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #2)
> Some social functionality depends on the livelihood of the worker (i.e.
> whether the worker has actually responded, rather than just whether we had
> attempted to create it and were queuing messages for it). I envisioned this
> as the mechanism for client code to detect that - for example, to only show
> the "share" button when we knew that the message it sends will be received.

I'm not sure that would make much sense for the "share" button, as we need to get an image from the provider.  I guess I don't really see what this tells us - eg, just because it responds with this message doesn't seem to imply it will certainly respond to a social.user-recommend-prompt message.

I'm happy if you want this closed, but I'd think that means we should open new bugs for places where workerapi.initialized should be checked but currently is not - and TBH I can't think of where that would be (it is currently not used *anywhere* by the runtime code)
The addition of this message pre-dates the user-recommend-prompt stuff (bug 780987). I'm not objecting to the removal, I just wanted to make it clear why this exists. Given that there's a suitable alternative now, I think you're right that removing it makes sense.
Comment on attachment 658339 [details] [diff] [review]
Remove the message

I think this is fine to remove until we know we have a use for it.
Attachment #658339 - Flags: feedback?(mixedpuppy) → feedback+
Comment on attachment 658339 [details] [diff] [review]
Remove the message

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

::: toolkit/components/social/test/browser/browser_workerAPI.js
@@ -30,1 @@
>    

nit: can you also remove this extra whitespace?
Attachment #658339 - Flags: review?(jaws) → review+
(Assignee)

Comment 7

5 years ago
Created attachment 660290 [details] [diff] [review]
updated tests

New patch with trivial change in the tests, carrying r+ forward.

https://hg.mozilla.org/integration/mozilla-inbound/rev/df2ddcab7143
Attachment #658339 - Attachment is obsolete: true
Attachment #660290 - Flags: review+
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/f496fceb64ad for xpcshell test_SocialService.js bustage.
(Assignee)

Comment 9

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a1b8c9f1184
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/4a1b8c9f1184
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
Attachment #660290 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/fe8907cef50e
status-firefox17: --- → fixed
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.