Closed Bug 629665 ([regression]) Opened 9 years ago Closed 9 years ago
Tab bar covers full screen
My tab bar expanded to cover all the screen. Mozilla/5.0 (Android; Linux armv7l; rv:2.0b11pre) Gecko/20110127 Firefox/4.0b11pre Fennec/4.0b5pre ID:20110127162904
This bug is a regression from bug 628732. I've done a mistake by substracting the height of the undo tab box. This patch fixes that and also fix some funny bugs encountered when adding tests, like: * the scroll position can be wrong when the undo tab is removed from the content and the sidebar is hidden * the scroll position can be wrong when a tab is closed (which is the reason for the new event, that fire _after_ the tab has been removed from the DOM)
hmm, I can probably move + event = document.createEvent("Events"); + event.initEvent("TabClosed", true, false); + window.dispatchEvent(event); before the this.selectedTab call!
9 years ago
9 years ago
Mark, if you don't like the TabClosed event I can remove it from this patch and open an other bug for it.
Can we drop "TabClosed" and use "TabClose", but setTimeout(..., 0) in the event handler. That should allow the teardown of the Tab to complete and should act like a "TabClosed"
(In reply to comment #4) > Can we drop "TabClosed" and use "TabClose", but setTimeout(..., 0) in the event > handler. That should allow the teardown of the Tab to complete and should act > like a "TabClosed" My problem the tab.chromeTab element (on which the event is dispatched) would be unexistent at this point :/ Another possibility is to rename the TabClose event TabWillClose and fired a new TabClosed event on the window? This is basically the same as what i already do in the patch except the name is closer to the truth.
I want "TabClose" to work like it does on desktop
This is not adding any new event. I'm just keeping a reference to the dom element to have it alive for the duration of the function.
Comment on attachment 509380 [details] [diff] [review] Patch v0.2 We are changing the way TabClose works here. The "browser" is now dead by the time TabClose is fired. The chromeTab is now removed from the document too. I'd worry that the event wouldn't even fire correctly, since the element is no longer in the DOM. I think this will break: http://mxr.mozilla.org/mobile-browser/source/components/SessionStore.js#212 which wants access to the "browser" Let me think a bit more about the previous idea of adding a "TabRemove" or something event.
Let's go with your first approach, but call the event "TabRemove"
(In reply to comment #9) > Let's go with your first approach, but call the event "TabRemove" Ok, I will update the patch during landing.
Before landing I'm re-asking review to be sure we agree on the solution.
Duplicate of this bug: 609600
Duplicate of this bug: 630277
Comment on attachment 509474 [details] [diff] [review] Patch v0.2 > Elements.tabs.addEventListener("TabSelect", this, true); > Elements.tabs.addEventListener("TabOpen", this, true); >+ window.addEventListener("TabRemove", this, true); I think we should fire the event on the chromeTab or the parent of the chrometab, so Elements.tabs.addEventListener should work for "TabRemove" too > event.initEvent("TabClose", true, false); > tab.chromeTab.dispatchEvent(event); > tab.browser.messageManager.sendAsyncMessage("Browser:TabClose"); > > tab.destroy(); > this._tabs.splice(tabIndex, 1); > > this.selectedTab = nextTab; >+ >+ event = document.createEvent("Events"); >+ event.initEvent("TabRemove", true, false); >+ window.dispatchEvent(event); Hold onto the chromeTab until the end of this function and fire the TabRemove event on it, or if that won't work, use the parent of the chromeTab. But let's not use the window. r+ with that change
Attachment #509474 - Flags: review?(mark.finkle) → review+
for Mozilla /5.0 (Maemo;Linux armv7l;rv:2.0b12pre)Gecko/20110204 Firefox/4.0b12pre Fennec /4.0b5pre Build ID: 20110204004947, Platform: Linux-maemo, Operating System: Linux-arm having three tabs and closing one of them, will double the tab panel.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Build Identifier: Mozilla/5.0 (Android; Linux armv7l; rv:2.0b8pre) Gecko/20101104 Firefox/4.0b8pre Fennec/4.0b3pre Device: HTC Desire The columns are not duplicated anymore, so I will set the bug to Verified Fixed
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.