Closed Bug 702796 Opened 13 years ago Closed 12 years ago

IndexedDB prompt should auto-dismiss after a timeout [in native fennec]

Categories

(Firefox for Android Graveyard :: General, defect, P3)

All
Android
defect

Tracking

(fennec+)

RESOLVED FIXED
Firefox 21
Tracking Status
fennec + ---

People

(Reporter: mbrubeck, Assigned: kshriram18)

References

Details

(Whiteboard: [mentor=mbrubeck@mozilla.com][good first bug][lang=js][lang=java])

Attachments

(2 files, 2 obsolete files)

IndexedDB prompts in desktop Firefox and XUL Fennec will automatically time out and disappear after a 5-minute timer (see bug 595253 for more explanation). We probably want to implement this in native Fennec too, for the same reason (so content doesn't wait indefinitely for a response from the prompt).
Priority: -- → P3
tracking-fennec: --- → 11+
Assignee: mbrubeck → nobody
Whiteboard: [mentor=mbrubeck@mozilla.com][good first bug]
Whiteboard: [mentor=mbrubeck@mozilla.com][good first bug] → [mentor=mbrubeck@mozilla.com][good first bug][lang=js]
Assignee: nobody → michaelkohler
Assignee: michaelkohler → nobody
You can look at the patches in bug 701527 to see where the code for this lives. In particular, the NativeWindow and IndexedDB objects in browser.js: https://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js
Whiteboard: [mentor=mbrubeck@mozilla.com][good first bug][lang=js] → [mentor=mbrubeck@mozilla.com][good first bug][lang=js][lang=java]
tracking-fennec: 11+ → +
I think this prompt is broken currently on Fennec? I'll try to get a better testcase for this, but currently my IndexedDB-using drawing app hangs after I create a certain number of drawings. By hangs I mean, the request=idb.open('<new database name>', version) call completes, but none of the callbacks (request.onsuccess, request.onerror, request.onblocked) are even invoked when I'm creating the 8th or so new database. All future database opens (even to already existing databases) also block in the same way (no callbacks are ever invoked). I'm speculating that what it happening is that the idb permissions dialog is being invoked but failing to ever become visible and so all the rest of the idb operations are waiting for the user to click on the invisible dialog.
I've opened bug 805560 for the "prompt is broken" problem described in comment 2. This bug should also stay open, for the "prompt should time out once it is unbroken" part of the issue.
Attachment #696678 - Flags: feedback? → feedback?(mbrubeck)
Comment on attachment 696678 [details] [diff] [review] Patch for IndexedDB prompt to auto-dismiss after a timeout[in native fennec] Review of attachment 696678 [details] [diff] [review]: ----------------------------------------------------------------- Good work! This is definitely the correct approach to port this code from desktop Firefox to Fennec. It looks like there are a few places where the code is not fully ported to Fennec APIs, which I tried to note below. With some minor changes, you should be able to build and test this patch and upload a new version for final review. ::: mobile/android/chrome/content/browser.js @@ +5570,5 @@ > + > + function timeoutNotification() { > + // Remove the notification. > + if (notification) { > + notification.remove(); Instead of "notification.remove" I think you'll need to call NativeWindow.doorhanger.hide. @@ +5579,5 @@ > + clearTimeout(timeoutId); > + > + // And tell the page that the popup timed out. > + observer.observe(null, responseTopic, > + Ci.nsIPermissionManager.UNKNOWN_ACTION); You can put this all on one line. (In Fennec code we tend not to wrap lines unless they are longer than around 120 characters.) @@ +5586,2 @@ > if (topic == this._quotaCancel) { > + notification = NativeWindow.doorhanger.hide(notificationID, tab.id); I don't think you should assign anything to "notification" here. NativeWindow.doorhanger.hide() does not return a value. @@ +5622,5 @@ > + > + // If the popup is being dismissed start the short timeout. > + if (state == "dismissed") { > + clearTimeout(timeoutId); > + timeoutId = setTimeout(timeoutNotification, hiddenTimeoutDuration); I don't think this "eventCallback" code is needed in Fennec. Unlike desktop Firefox, Fennec has no way to hide and then restore a doorhanger notification, though we might add one in the future (bug 700437). For now, I think you can just remove the whole "options" section of your patch. @@ +5636,5 @@ > + //timeout = > + }; > + > + notification = NativeWindow.doorhanger.show(message, notificationID, > + buttons, tab.id, options); NativeWindow.doorhanger.show does not return an object either. Instead of saving a "notification" variable, you can just use the notificationID and tab.id to dismiss the doorhanger later.
Attachment #696678 - Flags: feedback?(mbrubeck) → feedback+
Addresses points in last comment. will check if there's anything else to add
Attachment #696678 - Attachment is obsolete: true
Attachment #703747 - Flags: feedback?(mbrubeck)
Comment on attachment 703747 [details] [diff] [review] Patch for IndexedDB prompt to auto-dismiss after a timeout [in native fennec] Looks good, thanks! I think this is ready to check in with just two minor changes: >+ const hiddenTimeoutDuration = 30000; // 30 seonds You can remove this line because it is unused. >+ const firstTimeoutDuration = 360000; // 5 minutes I think this should be 300000 instead. (I see that desktop Firefox has the same mistake; want to submit a separate patch to fix that?) I can make these changes myself before checking in the patch.
Attachment #703747 - Flags: review+
Attachment #703747 - Flags: feedback?(mbrubeck)
Attachment #703747 - Flags: checkin?(mbrubeck)
Attachment #703747 - Attachment is obsolete: true
Attachment #703747 - Flags: checkin?(mbrubeck)
Addresses the timeout duration value in comment #7
Comment on attachment 709397 [details] [diff] [review] Patch that fixes timeout duration value Gavin, this contains a trivial fix for a constant in desktop Firefox.
Attachment #709397 - Flags: review?(gavin.sharp)
Attachment #709395 - Flags: review+
Attachment #709397 - Flags: review?(gavin.sharp) → review+
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: