Last Comment Bug 702386 - Support setting persistence to -1 for doorhangers
: Support setting persistence to -1 for doorhangers
Status: RESOLVED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All Android
: P2 normal (vote)
: ---
Assigned To: :Margaret Leibovic
:
Mentors:
Depends on:
Blocks: 700913
  Show dependency treegraph
 
Reported: 2011-11-14 12:03 PST by :Margaret Leibovic
Modified: 2012-01-09 11:56 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
11+


Attachments
patch (3.14 KB, patch)
2011-11-14 14:22 PST, :Margaret Leibovic
mark.finkle: review+
Details | Diff | Review

Description :Margaret Leibovic 2011-11-14 12:03:16 PST
Follow-up to 700913, since I missed it. If persistence is set to -1, the doorhanger never closes automatically, but must be closed by the user.
Comment 1 :Margaret Leibovic 2011-11-14 14:22:34 PST
Created attachment 574422 [details] [diff] [review]
patch

When I was writing this patch I realized that my previous patch was actually buggy - there was no way to override the persistence or timeout options, even in cases where you always want to remove the doorhanger, like the onClick handler for the buttons. I modeled my fix after removeTransientNotifications in notification.xml, so now there's a separate remove method to handle this special location change case.
Comment 2 Mark Finkle (:mfinkle) (use needinfo?) 2011-11-14 18:52:08 PST
Comment on attachment 574422 [details] [diff] [review]
patch

>     public boolean shouldRemove() {
>-        if (mPersistence > 0) {
>+        if (mPersistence != 0) {

Maybe add a comment about -1 being a legal value meaning never automatically close?
Comment 3 :Margaret Leibovic 2011-11-15 09:24:42 PST
(In reply to Mark Finkle (:mfinkle) from comment #2)
> Comment on attachment 574422 [details] [diff] [review] [diff] [details] [review]
> patch
> 
> >     public boolean shouldRemove() {
> >-        if (mPersistence > 0) {
> >+        if (mPersistence != 0) {
> 
> Maybe add a comment about -1 being a legal value meaning never automatically
> close?

Good call - the toolkit code didn't have that comment, and that's why I copied it wrong! :)
Comment 4 :Margaret Leibovic 2011-11-15 12:32:30 PST
https://hg.mozilla.org/projects/birch/rev/2c4d1a8d4079

Note You need to log in before you can comment on or make changes to this bug.