java.lang.IllegalArgumentException: Path must not be empty.

RESOLVED FIXED in Firefox 30



5 years ago
5 years ago


(Reporter: Margaret, Assigned: Margaret)


(Blocks 1 bug)

Firefox 31
Dependency tree / graph

Firefox Tracking Flags

(firefox29 unaffected, firefox30 fixed, firefox31 fixed, fennec30+)


(crash signature)


(1 attachment)



5 years ago
I ran into this on a local build (today's m-c) with my hub kitchen sink add-on:

E/GeckoAppShell(25510): java.lang.IllegalArgumentException: Path must not be empty.
E/GeckoAppShell(25510): 	at com.squareup.picasso.Picasso.load(
E/GeckoAppShell(25510): 	at org.mozilla.gecko.home.PanelLayout.deliverDataset(
E/GeckoAppShell(25510): 	at org.mozilla.gecko.home.DynamicPanel$PanelLoaderCallbacks.onLoadFinished(
E/GeckoAppShell(25510): 	at$LoaderInfo.callOnLoadFinished(
E/GeckoAppShell(25510): 	at$LoaderInfo.onLoadComplete(
E/GeckoAppShell(25510): 	at
E/GeckoAppShell(25510): 	at org.mozilla.gecko.home.SimpleCursorLoader.deliverResult(
E/GeckoAppShell(25510): 	at org.mozilla.gecko.home.SimpleCursorLoader.deliverResult(
E/GeckoAppShell(25510): 	at
E/GeckoAppShell(25510): 	at$LoadTask.onPostExecute(
E/GeckoAppShell(25510): 	at
E/GeckoAppShell(25510): 	at$500(
E/GeckoAppShell(25510): 	at$InternalHandler.handleMessage(
E/GeckoAppShell(25510): 	at android.os.Handler.dispatchMessage(
E/GeckoAppShell(25510): 	at android.os.Looper.loop(
E/GeckoAppShell(25510): 	at
E/GeckoAppShell(25510): 	at java.lang.reflect.Method.invokeNative(Native Method)
E/GeckoAppShell(25510): 	at java.lang.reflect.Method.invoke(
E/GeckoAppShell(25510): 	at$
E/GeckoAppShell(25510): 	at
E/GeckoAppShell(25510): 	at dalvik.system.NativeStart.main(Native Method)


5 years ago
Crash Signature: java.lang.IllegalArgumentException: Path must not be empty.
Picasso is fine with null image URLs but not with empty strings. Maybe we should avoid saving this kind of data altogether here?

Comment 2

5 years ago
Yeah, that sounds like a good idea.

I just noticed this again and looked into what was causing it, and it was this:

    optionsCallback: function () {
      return {
        title: "Test Empty",
        views: [{
          type: Home.panels.View.LIST,
          dataset: "does.not.exist",
          empty: {
            text: "This is some test emtpy text",
            imageUrl: ""

So we should also make sure to bail appropriately if add-ons use empty strings for other images.


5 years ago
Assignee: nobody → margaret.leibovic
Priority: -- → P1

Comment 3

5 years ago
We actually handle this for panel items with a TextUtils.isEmpty check before making the Picasso call:

I did a quick audit of where we use Picasso.with, and in addition to the empty view in PanelLayout, we could also run into this problem with PanelBackItemView.

While we could add a check higher up the stream to make sure we don't have an empty string for these values, adding a TextUtils.isEmpty check before using the Picasso API seems safest.

Comment 4

5 years ago
I updated PanelBackItemView and PanelLayout to do what we do in PanelAuthLayout.
Attachment #8411362 - Flags: review?(
Comment on attachment 8411362 [details] [diff] [review]
Never pass an empty image path to Picasso

Review of attachment 8411362 [details] [diff] [review]:

::: mobile/android/base/home/
@@ +36,5 @@
> +            Picasso.with(getContext())
> +                   .load(backImageUrl)
> +                   .placeholder(R.drawable.folder_up)
> +                   .into(image);
> +        }

This looks fine but wouldn't it better to just lean on Picasso for the placeholder logic? Something like:

       .load(TextUtils.isEmpty(backImageUrl) ? null : backImageUrl),

I don't feel strongly about it. Up to you.

::: mobile/android/base/home/
@@ +442,5 @@
>              } else {
>                  Picasso.with(getContext())
>                         .load(imageUrl)
>                         .error(R.drawable.icon_home_empty_firefox)
>                         .into(imageView);

Attachment #8411362 - Flags: review?( → review+

Comment 6

5 years ago
When working on the empty views, we discussed avoiding the placeholder because we would see a default image flash before the real image was loaded. However, when I tried using an error image instead of a placeholder, I saw a lag before the default image is displayed while Picasso tries to load the null URL (perhaps there's room for improvement in Picasso there). In any case, I found that this approach of directly setting the default image ourselves performed best.


5 years ago
tracking-fennec: ? → 30+

Comment 7

5 years ago

I'll request uplift once we also uplift the empty views stuff.
Flags: needinfo?(margaret.leibovic)

Comment 8

5 years ago
Comment on attachment 8411362 [details] [diff] [review]
Never pass an empty image path to Picasso

Note to release managers: I'm requesting uplift for a series of Firefox Hub bugs. The main fixes we need are in bugs at the end of the series, but trying to rebase those patches proved difficult and risky, so I think we should just uplift the dependecies.

Note to sheriffs: I have a local patch series that I can land on aurora when these bugs all get approval.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): dependency for initial Firefox Hub release (promoted feed add-ons in fx30)
User impact if declined: panels won't update when data changes
Testing completed (on m-c, etc.): just landed on fx-team
Risk to taking this patch (and alternatives if risky): only affects dynamic panels, need an add-on to trigger this
String or IDL/UUID changes made by this patch: none
Attachment #8411362 - Flags: approval-mozilla-aurora?
Flags: needinfo?(margaret.leibovic)
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
Attachment #8411362 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 11

5 years ago
Setting P1 hub bugs to block hub v1 EPIC bug (targeting fx30 release).

Filter on epic-hub-bugs.
Blocks: 1014025
You need to log in before you can comment on or make changes to this bug.