Closed Bug 701305 Opened 13 years ago Closed 13 years ago

Refactor DoorHanger code to make it easier to support persistence/timeout options

Categories

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

All
Android
defect

Tracking

(firefox11 fixed, fennec11+)

RESOLVED FIXED
Tracking Status
firefox11 --- fixed
fennec 11+ ---

People

(Reporter: Margaret, Assigned: Margaret)

References

Details

Attachments

(1 file)

Attached patch patch (deleted) — Splinter Review
The current remove/hide code for doorhanger notifications is spread around in a way that makes it hard to try to implement bug 700913. I based these changes off the way we handle doorhangers in PopupNotifications.jsm, and it should be easy to add persistence/timeout options on top of this patch. Basically, I decided that we should just maintain a single set of doorhangers for each tab, and then we can let one method in DoorHangerPopup (updatePopupForTab) handle showing/hiding these doorhangers as appropriate. I also moved the code that creates DoorHangers to DoorHangerPopup.addDoorHanger so that GeckoApp doesn't need to do as much.
Attachment #573441 - Flags: review?(sriram)
Attachment #573441 - Flags: review?(mark.finkle)
Attachment #573441 - Attachment is patch: true
Also, I removed unnecessary calls to removeAllDoorhangers() in Tab, since handleLocationChange takes care of those cases for us (although I need to double check about reload).
Comment on attachment 573441 [details] [diff] [review] patch >diff --git a/mobile/chrome/content/browser.js b/mobile/chrome/content/browser.js > doorhanger: { >- show: function(aMessage, aValue, aButtons, aTab) { >- // use the current tab if none is provided >- let tabID = aTab ? aTab : BrowserApp.selectedTab.id; >+ >+ show: function(aMessage, aValue, aButtons, aTabID) { One of the refactors I want to do is make the NativeWindow API use "Tab" objects and not the Tab.id". I think it feels more natural taht way. Internally, the API will send the ID over to Java, but we shouldn't need to force the JS side to deal with tabIDs. We can do that in a NativeWindow API refactor though. Nice cleanup
Attachment #573441 - Flags: review?(mark.finkle) → review+
Assignee: nobody → margaret.leibovic
Attachment #573441 - Flags: review?(sriram)
Priority: -- → P3
This depends on the patches from bug 698598 that were recently backed out, so I'm gonna wait until those re-land to land this.
Depends on: 698598
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
tracking-fennec: --- → 11+
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: