Closed Bug 1035171 Opened 7 years ago Closed 7 years ago
Switching tabs triggers on
Location Change with LOCATION _CHANGE _SAME _DOCUMENT flag
SeaMonkey's implementation of web progress listeners for the tabbrowser widget behaves differently from the one in Firefox. When the user switches to another tab both implementations will trigger onLocationChange listeners. However, Firefox calls it without any flags whereas SeaMonkey will always use nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT flag. This makes no sense whatsoever since the new URLs clearly belongs to a different document. This issue was apparently introduced in bug 782516 but I cannot mark this bug as blocking it - bug 782516 is still marked as sensitive for some reason. Details: tabbrowser.updateCurrentBrowser() is called when switching to another tab. It calls tabbrowser.updateUrlBar() with nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT as aFlags parameter. And tabbrowser.updateUrlBar() will in turn forward that parameter to the onLocationBar method in web progress listeners.
(In reply to bug 782516 comment #8) > >+ if (this.popupNotifications && > >+ !(aFlags & nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT)) > > this.popupNotifications.locationChange(); > Oops, this doesn't quite work, because it triggers when you switch tabs. A > bit of a hacky workaround is to get the tab-switching code to send the > LOCATION_CHANGE_SAME_DOCUMENT flag in this case. This code was then removed in bug 843800, so the hack is no longer necessary.
Hi Wladimir! Is there anything else to this bug? Otherwise I'm going to close this.
Why close? The way I understand comment 1 nothing prevents fixing this bug. As things are now, Adblock Plus functionality is broken because SeaMonkey is lying to it - and I see no way to work around this.
(In reply to Wladimir Palant from comment #3) > Why close? The way I understand comment 1 nothing prevents fixing this bug. > As things are now, Adblock Plus functionality is broken because SeaMonkey is > lying to it - and I see no way to work around this. Somebody who understands this code should submit a patch to remove the hack. > This code was then removed in bug 843800, so the hack is no longer necessary.
> This code was then removed in bug 843800, so the hack is no longer necessary.
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED
Attachment #8466986 - Flags: review?(neil)
comm-central trees are closed at the moment. But will push when it reopens.
Comment on attachment 8466986 [details] [diff] [review] Patch v1.0 don't send LOCATION_CHANGE_SAME_DOCUMENT [Approval Request Comment] Regression caused by (bug #): Bug 782516  User impact if declined: AdblockPlus can't function properly. Probably also affects other content blocker extensions. Testing completed (on m-c, etc.): been cooking in my local tree for a month. Risk to taking this patch (and alternatives if risky): very low risk regression fix. String changes made by this patch: None.  Bug 782516 [tabbrowser.xml] onLocationChange: aRequest can be null for an error page.
Attachment #8466986 - Flags: approval-comm-release?
Attachment #8466986 - Flags: approval-comm-release+
Attachment #8466986 - Flags: approval-comm-beta?
Attachment #8466986 - Flags: approval-comm-beta+
Attachment #8466986 - Flags: approval-comm-aurora?
Attachment #8466986 - Flags: approval-comm-aurora+
Pushed to comm-central: http://hg.mozilla.org/comm-central/rev/e78fb70d9419 Pushed to branches: https://hg.mozilla.org/releases/comm-aurora/rev/41c5afb2089e https://hg.mozilla.org/releases/comm-beta/rev/ec9d0275b763 Pushed to comm-release (2.29.1) https://hg.mozilla.org/releases/comm-beta/rev/ec9d0275b763
You need to log in before you can comment on or make changes to this bug.