After bug 1224521 we have replaced all toasts from JavaScript (browser.js) and calls to the toast method in BrowserApp. There are still calls to Toast.makeText(..).show() scattered all over our code base. This bug is about finding and replacing them with snackbars. This might be a bit harder: Toasts only needed a context. Snackbars need a view to find the root layout. https://dxr.mozilla.org/mozilla-central/search?q=Toast.makeText&redirect=true&case=false

Alex: Maybe you want to continue with the toasts here. Some of them might be hard or impossible to replace. Start with the easy ones.

(In reply to Sebastian Kaspari (:sebastian) from comment #1) > Alex: Maybe you want to continue with the toasts here. Some of them might be > hard or impossible to replace. Start with the easy ones. Yeah, I'd pick this up. Could you assign it to me?

Bug 1228414 - Replaced Toaster.makeText() calls in GeckoApp and BrowserApp r?sebastian

Bug 1228414 - Replaced clear private data toast with snackbar r?sebastian

Bug 1228414 - Replaced the last search engine toast. r?sebastian

Comment on attachment 8693225 [details] MozReview Request: Bug 1228414 - Replaced all the possible Toast.makeTexts with Snackbars. r+sebastian https://reviewboard.mozilla.org/r/26413/#review23891 Good job! r+ with the changes mentioned below. ::: mobile/android/base/GeckoApp.java:1063 (Diff revision 1) > + > + String set_image_path_fail = getResources().getString(R.string.set_image_path_fail); > + > if (!dcimDir.mkdirs() && !dcimDir.isDirectory()) { > - Toast.makeText((Context) this, R.string.set_image_path_fail, Toast.LENGTH_SHORT).show(); > + showSnackbar(set_image_path_fail, Snackbar.LENGTH_SHORT, null, null); > return; > } > String path = Media.insertImage(getContentResolver(),image, null, null); > if (path == null) { > - Toast.makeText((Context) this, R.string.set_image_path_fail, Toast.LENGTH_SHORT).show(); > + showSnackbar(set_image_path_fail, Snackbar.LENGTH_SHORT, null, null); It's okay to use getString() inside showSnackbar. No need to create this String early. Most of the time I might not be needed. ::: mobile/android/base/GeckoApp.java:1064 (Diff revision 1) > + String set_image_path_fail = getResources().getString(R.string.set_image_path_fail); It looks like this String is not used? ::: mobile/android/base/GeckoApp.java:1089 (Diff revision 1) > - Toast.makeText((Context) this, R.string.set_image_fail, Toast.LENGTH_SHORT).show(); > + showSnackbar(getResources().getString(R.string.set_image_fail), Snackbar.LENGTH_SHORT, null, null); NIT: In an activity you can just call getString() without getResources().

Comment on attachment 8693225 [details] MozReview Request: Bug 1228414 - Replaced all the possible Toast.makeTexts with Snackbars. r+sebastian https://reviewboard.mozilla.org/r/26413/#review23893 ::: mobile/android/base/GeckoApp.java:2274 (Diff revision 1) > - Toast.makeText(this, message, Toast.LENGTH_LONG).show(); > + showSnackbar(message, Snackbar.LENGTH_LONG, null, null); This needs to be a toast. It's shown if the current system is not supported. We finish the activity immediately in this situation. So there's no UI to show a snackbar.

Comment on attachment 8693225 [details] MozReview Request: Bug 1228414 - Replaced all the possible Toast.makeTexts with Snackbars. r+sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26413/diff/1-2/

Comment on attachment 8693226 [details] MozReview Request: Bug 1228414 - Replaced clear private data toast with snackbar r?sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26407/diff/1-2/

Comment on attachment 8693227 [details] MozReview Request: Bug 1228414 - Replaced the last search engine toast. r?sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26415/diff/1-2/

Comment on attachment 8693226 [details] MozReview Request: Bug 1228414 - Replaced clear private data toast with snackbar r?sebastian https://reviewboard.mozilla.org/r/26407/#review23901 ::: mobile/android/base/preferences/GeckoPreferences.java:626 (Diff revision 1) > - Toast.makeText(context, stringRes, Toast.LENGTH_SHORT).show(); > + final FragmentManager fragmentManager = getFragmentManager(); > + int id = getResources().getIdentifier("android:id/prefs", null, null); > + final Fragment fragment = fragmentManager.findFragmentById(id); > + Snackbar.make(fragment.getView(), stringRes, Snackbar.LENGTH_SHORT).show(); This probably does not work on Gingerbread where we do not use fragments - can you test this? Maybe you can use the root view (android.R.id.content) instead.

Comment on attachment 8693227 [details] MozReview Request: Bug 1228414 - Replaced the last search engine toast. r?sebastian https://reviewboard.mozilla.org/r/26415/#review23903 ::: mobile/android/base/preferences/SearchEnginePreference.java:97 (Diff revision 2) > + Fragment fragment = activity.getFragmentManager() > + .findFragmentById(getContext().getResources() > + .getIdentifier("android:id/prefs", null, null)); > + > + Snackbar.make(fragment.getView(), R.string.pref_search_last_toast, Snackbar.LENGTH_SHORT).show(); Like in the previous commit: I assume this does not work on Gingerbread.

Comment on attachment 8693225 [details] MozReview Request: Bug 1228414 - Replaced all the possible Toast.makeTexts with Snackbars. r+sebastian https://reviewboard.mozilla.org/r/26413/#review23905

Comment on attachment 8693226 [details] MozReview Request: Bug 1228414 - Replaced clear private data toast with snackbar r?sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26407/diff/2-3/

Comment on attachment 8693227 [details] MozReview Request: Bug 1228414 - Replaced the last search engine toast. r?sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26415/diff/2-3/

Bug 1228414 - Replaced EditBookmark toast with snackbar. r?sebastian

Bug 1228414 - Replaced toasts in GeckoAppShell and HomeFragment r?sebastian

Comment on attachment 8693226 [details] MozReview Request: Bug 1228414 - Replaced clear private data toast with snackbar r?sebastian https://reviewboard.mozilla.org/r/26407/#review24137 ::: mobile/android/base/preferences/GeckoPreferences.java:626 (Diff revision 3) > - Toast.makeText(context, stringRes, Toast.LENGTH_SHORT).show(); > + Snackbar.make(findViewById(android.R.id.content), stringRes, Snackbar.LENGTH_SHORT).show(); So this worked? Did you test this on Gingerbread too?

Comment on attachment 8693227 [details] MozReview Request: Bug 1228414 - Replaced the last search engine toast. r?sebastian https://reviewboard.mozilla.org/r/26415/#review24139

Comment on attachment 8693621 [details] MozReview Request: Bug 1228414 - Replaced EditBookmark toast with snackbar. r?sebastian https://reviewboard.mozilla.org/r/26505/#review24141 ::: mobile/android/base/EditBookmarkDialog.java:220 (Diff revision 1) > - Toast.makeText(context, R.string.bookmark_updated, Toast.LENGTH_SHORT).show(); > + View view = ((Activity)context).findViewById(R.id.root_layout); > + Snackbar.make(view, R.string.bookmark_updated, Snackbar.LENGTH_SHORT).show(); "root_layout" is going away eventually (bug 1107636). So we better do not rely on it anymore. You are assuming that the activity is using a layout with "root layout". This means you are basically saying that this is the BrowserApp activity. So I wonder if we should just cast to BrowserActivity and call showSnackbar().

Comment on attachment 8693622 [details] MozReview Request: Bug 1228414 - Replaced toasts in GeckoAppShell and HomeFragment r?sebastian https://reviewboard.mozilla.org/r/26507/#review24143 ::: mobile/android/base/GeckoAppShell.java:2601 (Diff revision 1) > - Toast toast = Toast.makeText(getContext(), > - getContext().getResources().getString(R.string.share_image_failed), > + View rootView = ((Activity)getContext()).findViewById(R.id.root_layout); > + Snackbar.make(rootView, R.string.share_image_failed, Snackbar.LENGTH_SHORT).show(); See previous review. ::: mobile/android/base/home/HomeFragment.java:408 (Diff revision 1) > - Toast.makeText(mContext, R.string.page_removed, Toast.LENGTH_SHORT).show(); > + View rootView = ((Activity)mContext).findViewById(R.id.root_layout); > + Snackbar.make(rootView, R.string.page_removed, Snackbar.LENGTH_SHORT).show(); see above

https://reviewboard.mozilla.org/r/26407/#review24137 > So this worked? Did you test this on Gingerbread too? It worked on my phone, but I'm having an exceptionally hard time building Fennec for Gingerbread.

(In reply to Alex Johnson(:alex_johnson) from comment #22) > https://reviewboard.mozilla.org/r/26407/#review24137 > > > So this worked? Did you test this on Gingerbread too? > > It worked on my phone, but I'm having an exceptionally hard time building > Fennec for Gingerbread. You should be able to run a normal Fennec build on Gingerbread. You get closer to a release Gingerbread build with the following things set in your mozconfig: ac_add_options --with-android-min-sdk=9 ac_add_options --with-android-max-sdk=10 ac_add_options --enable-android-resource-constrained Note that with this config you can build with 'mach' but you can't build with Android Studio / IntelliJ

Comment on attachment 8693225 [details] MozReview Request: Bug 1228414 - Replaced all the possible Toast.makeTexts with Snackbars. r+sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26413/diff/2-3/

Comment on attachment 8693226 [details] MozReview Request: Bug 1228414 - Replaced clear private data toast with snackbar r?sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26407/diff/3-4/

Comment on attachment 8693227 [details] MozReview Request: Bug 1228414 - Replaced the last search engine toast. r?sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26415/diff/3-4/

Comment on attachment 8693621 [details] MozReview Request: Bug 1228414 - Replaced EditBookmark toast with snackbar. r?sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26505/diff/1-2/

Comment on attachment 8693622 [details] MozReview Request: Bug 1228414 - Replaced toasts in GeckoAppShell and HomeFragment r?sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26507/diff/1-2/

https://reviewboard.mozilla.org/r/26505/#review24141 > "root_layout" is going away eventually (bug 1107636). So we better do not rely on it anymore. > > You are assuming that the activity is using a layout with "root layout". This means you are basically saying that this is the BrowserApp activity. So I wonder if we should just cast to BrowserActivity and call showSnackbar(). Casting to BrowserActivity isn't working because the mLayout in GeckoApp can not be accessed from the EditBookmark activity. So, I decided to just switch to android.R.id.content.

Comment on attachment 8693621 [details] MozReview Request: Bug 1228414 - Replaced EditBookmark toast with snackbar. r?sebastian https://reviewboard.mozilla.org/r/26505/#review24719

Comment on attachment 8693622 [details] MozReview Request: Bug 1228414 - Replaced toasts in GeckoAppShell and HomeFragment r?sebastian https://reviewboard.mozilla.org/r/26507/#review24723

Hi, this failed to apply: apply changeset? [ynmpcq?]: y applying 1772a6c68b74 unable to find 'mobile/android/base/BrowserApp.java' for patching 4 out of 4 hunks FAILED -- saving rejects to file mobile/android/base/BrowserApp.java.rej unable to find 'mobile/android/base/GeckoApp.java' for patching 3 out of 3 hunks FAILED -- saving rejects to file mobile/android/base/GeckoApp.java.rej patch failed to apply abort: fix up the merge and run hg transplant --continue can you take a look, thanks!

Comment on attachment 8693225 [details] MozReview Request: Bug 1228414 - Replaced all the possible Toast.makeTexts with Snackbars. r+sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26413/diff/3-4/

Fixed!

A snackbar is displayed after: > - a page is removed from reading list > - a bookmark is removed > - a link is bookmarked > - an already bookmark is added as bookmark > - a bookmark is updated > - a page is removed from home panels Verified as fixed using: Device: Nexus 6 (Android 6.0) Build: Firefox for Android 45.0a2 and 46.0a1 (2015-12-22) Leaving this RESOLVED FIXED until bug 1233412 gets fixed.