1161436 - return after return in browser/components/pocket/pktApi.js.

Status: RESOLVED FIXED

(Firefox :: Pocket, defect)

Description

[Tooru Fujisawa [:arai]]

https://dxr.mozilla.org/mozilla-central/source/browser/components/pocket/pktApi.js#296
https://dxr.mozilla.org/mozilla-central/source/browser/components/pocket/pktApi.js#325
>        return apiRequest({
>            path: "/firefox/save",
>            data: sendData,
>            success: function(data) {
>
>                // Update premium status, tags and since
>                var tags = data.tags;
>                if ((typeof tags !== "undefined") && Array.isArray(tags)) {
>                    // If a tagslist is in the response replace the tags
>                    setSetting('tags', JSON.stringify(data.tags));
>                }
>
>                // Update premium status
>                var premiumStatus = data.premium_status;
>                if (typeof premiumStatus !== "undefined") {
>                    // If a premium_status is in the response replace the premium_status
>                    setSetting("premium_status", premiumStatus);
>                }
>
>                // Save since value for further requests
>                setSetting('latestSince', data.since);
>
>                if (options.success) {
>                    options.success.apply(options, Array.apply(null, arguments));
>                }
>            },
>            error: options.error
>        });
>
>        return sendAction(action, options);

here, return is placed after return.
There is no variable named "action". can we just remove the second one? or is there any bug in first one?

Comment 1

[Tooru Fujisawa [:arai]]

One more unreachable code after return in pocket.

https://dxr.mozilla.org/mozilla-central/source/browser/components/pocket/main.js#379
>        return;
>
>    	// TODO : Animate the change if given options.animate = true
>    	getPanel().sizeTo(options.width, options.height);
>    	setTimeout(function(){
>    		// we set the iframe size directly because it does not automatically stretch vertically
>            var height = document.getElementById('pocket-panel-container').clientHeight + 'px';
>	    	getPanelFrame().style.height = height;
>	    },1);

this seems to be intentional.
but now we're showing warning, so it might be better to comment them out.

Comment 2

[Tooru Fujisawa [:arai]]

jaws, how do you think about them? especially for comment #0's case.

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

This is imported code from Pocket and we didn't want to change things that weren't necessary for the initial landing. We have new code to import and hopefully this will have been cleaned up.

I'll leave the needinfo flag on this bug to remind me to come back after the import and check again.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

This has been fixed in pktAPI.js now.