Closed
Bug 1216051
Opened 9 years ago
Closed 9 years ago
Deprecate NativeWindow.toast or delegate calls to Snackbar API
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox45 fixed)
RESOLVED
FIXED
Firefox 45
Tracking | Status | |
---|---|---|
firefox45 | --- | fixed |
People
(Reporter: sebastian, Assigned: alex_johnson)
References
Details
(Keywords: dev-doc-complete)
Attachments
(1 file, 1 obsolete file)
Bug 1215026 will introduce an API for showing Snackbars. After that we could: 1) Just deprecate NativeWindow.toast. 2) Delegate calls from NativeWindow.toast to the new Snackbar API. And optionally deprecate NativeWindow.toast. 3) Keep both around. Android hasn't deprecated Toasts so far.
Comment 1•9 years ago
|
||
Do we want to show both snackbars and toasts in our own UI? If we want to transition to only use snackbars, I think add-on APIs should go the same way. It's a bit unfortunate that the add-on API name maps so directly to the implementation. To avoid breaking existing add-ons, I'm leaning towards option 2, but I don't see a reason to change add-on behavior until we're ready to drop this API in our own UI. You should also be able to use your LDAP to access the addons MXR to see how often this API is used by addons hosted on AMO: https://mxr.mozilla.org/addons/
Comment 2•9 years ago
|
||
I like option 3. Android still allows the use of Toasts.
Reporter | ||
Comment 3•9 years ago
|
||
Bug 1157526 replaced all button toasts with snackbars. * Now we can deprecate at least toast buttons (bug 1224523) * Bug 1224521 is filed about replacing the remaining toasts. Let's see if we still use toasts after that bug or if we are going to replace all of them. Should we still see a use for toasts in the app then we probably should allow add-ons to use them too.
Assignee | ||
Comment 4•9 years ago
|
||
Bug 1216051 - Delegated NativeWindow.toast to Snackbar API. r?sebastian
Attachment #8692242 -
Flags: review?(s.kaspari)
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to Alex Johnson(:alex_johnson) from comment #4) > Created attachment 8692242 [details] > MozReview Request: Bug 1216051 - Delegated NativeWindow.toast to Snackbar > API. r?sebastian > > Bug 1216051 - Delegated NativeWindow.toast to Snackbar API. r?sebastian I think we should delegate NativeWindow.toast to Snackbar API since quite a few addons still use the NativeWindow API: https://mxr.mozilla.org/addons/search?string=NativeWindow.toast
Reporter | ||
Comment 6•9 years ago
|
||
Comment on attachment 8692242 [details] MozReview Request: Bug 1216051 - Delegated NativeWindow.toast to Snackbar API. r?sebastian https://reviewboard.mozilla.org/r/26243/#review23737 Nice clean up! ::: mobile/android/chrome/content/browser.js (Diff revision 1) > - _callbacks: {}, We should let callers know that this method is now deprecated. We already do this for NativeWindow.pageactions. Search for "deprecated" in browser.js :) ::: mobile/android/chrome/content/browser.js:2302 (Diff revision 1) > - // to jar:jar urls so that the frontend can show them > + options = { NIT: Instead of re-creating options we probably can just write to options.action = { ... } Nice
Attachment #8692242 -
Flags: review?(s.kaspari)
Reporter | ||
Updated•9 years ago
|
Attachment #8692242 -
Flags: feedback+
Reporter | ||
Updated•9 years ago
|
Assignee: nobody → alex_johnson24
Status: NEW → ASSIGNED
Assignee | ||
Updated•9 years ago
|
Attachment #8692242 -
Flags: feedback+ → review?(s.kaspari)
Assignee | ||
Comment 7•9 years ago
|
||
Comment on attachment 8692242 [details] MozReview Request: Bug 1216051 - Delegated NativeWindow.toast to Snackbar API. r?sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26243/diff/1-2/
Assignee | ||
Updated•9 years ago
|
Keywords: dev-doc-needed
Reporter | ||
Comment 8•9 years ago
|
||
Comment on attachment 8692242 [details] MozReview Request: Bug 1216051 - Delegated NativeWindow.toast to Snackbar API. r?sebastian https://reviewboard.mozilla.org/r/26243/#review23761 ::: mobile/android/chrome/content/browser.js:2291 (Diff revision 2) > - let msg = { > - type: "Toast:Show", > + let err = Strings.browser.formatStringFromName("nativeWindow.deprecated", ["NativeWindow.toast", "Snackbars"], 2); > + Cu.reportError(err); I think you do not need to add your own error messaging. It's probably enough to add it to the list of deprecated interfaces and the calls will automatically be wrapped: https://dxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#3136-3141 Can you try this?
Attachment #8692242 -
Flags: review?(s.kaspari)
Reporter | ||
Updated•9 years ago
|
Attachment #8692242 -
Flags: feedback+
Assignee | ||
Comment 9•9 years ago
|
||
https://reviewboard.mozilla.org/r/26243/#review23761 > I think you do not need to add your own error messaging. It's probably enough to add it to the list of deprecated interfaces and the calls will automatically be wrapped: > https://dxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#3136-3141 > > Can you try this? I did try that. But when I tested it, the calls to NativeWindow.toast would just show the error message and would not delegate to Snackbars.
Assignee | ||
Comment 10•9 years ago
|
||
https://reviewboard.mozilla.org/r/26243/#review23761 > I did try that. But when I tested it, the calls to NativeWindow.toast would just show the error message and would not delegate to Snackbars. I actually figured it out. I moved the check for string durations to Snackbars.jsm and removed the toast from browser.js. It seems to be working now.
Assignee | ||
Comment 11•9 years ago
|
||
Bug 1216051 - Moved the check for toast into Snackbars.jsm. r?sebastian
Attachment #8692959 -
Flags: review?(s.kaspari)
Reporter | ||
Comment 12•9 years ago
|
||
Comment on attachment 8692242 [details] MozReview Request: Bug 1216051 - Delegated NativeWindow.toast to Snackbar API. r?sebastian Ok, thanks! It looks like this only works if the API is moved 1:1 to a JSM file.
Attachment #8692242 -
Flags: review?(s.kaspari)
Reporter | ||
Comment 13•9 years ago
|
||
Comment on attachment 8692959 [details] MozReview Request: Bug 1216051 - Moved the check for toast into Snackbars.jsm. r?sebastian https://reviewboard.mozilla.org/r/26361/#review23791 Ah, nice! Just move this (see below) and then we are ready to land this! I'll do some local testing too. ::: mobile/android/modules/Snackbars.jsm:26 (Diff revision 1) > + // Takes care of the deprecated toast calls Let's move this to some function like migrateToastIfNeeded(). Just to keep the code simple here.
Attachment #8692959 -
Flags: review?(s.kaspari) → review+
Reporter | ||
Comment 14•9 years ago
|
||
Oh, and let's fold the two commits into one before landing. :)
Assignee | ||
Comment 15•9 years ago
|
||
Comment on attachment 8692242 [details] MozReview Request: Bug 1216051 - Delegated NativeWindow.toast to Snackbar API. r?sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26243/diff/2-3/
Attachment #8692242 -
Flags: feedback+
Assignee | ||
Updated•9 years ago
|
Attachment #8692959 -
Attachment is obsolete: true
Assignee | ||
Comment 16•9 years ago
|
||
Comment on attachment 8692242 [details] MozReview Request: Bug 1216051 - Delegated NativeWindow.toast to Snackbar API. r?sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26243/diff/3-4/
Reporter | ||
Updated•9 years ago
|
Attachment #8692242 -
Flags: review?(s.kaspari) → review+
Reporter | ||
Comment 17•9 years ago
|
||
Comment on attachment 8692242 [details] MozReview Request: Bug 1216051 - Delegated NativeWindow.toast to Snackbar API. r?sebastian https://reviewboard.mozilla.org/r/26243/#review23799 Awesome. Thank you!
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 18•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ab0a2f5a1d95
Comment 19•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/6614f51e468a
Keywords: checkin-needed
Comment 20•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6614f51e468a
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Assignee | ||
Updated•9 years ago
|
Keywords: dev-doc-needed
Comment 21•9 years ago
|
||
It looks like alex_johnson has already updated the NativeWindow.toast docs with this deprecation message. Thanks! So I'm going to mark this dev-doc-complete.
Keywords: dev-doc-complete
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•