Closed
Bug 1035171
Opened 7 years ago
Closed 7 years ago
Switching tabs triggers onLocationChange with LOCATION_CHANGE_SAME_DOCUMENT flag
Categories
(SeaMonkey :: Tabbed Browser, defect)
SeaMonkey
Tabbed Browser
Tracking
(seamonkey2.26 wontfix, seamonkey2.27 wontfix, seamonkey2.28 affected, seamonkey2.29 fixed, seamonkey2.30 fixed, seamonkey2.31 fixed, seamonkey2.32 fixed)
RESOLVED
FIXED
seamonkey2.32
People
(Reporter: ecfbugzilla, Assigned: philip.chee)
References
Details
Attachments
(1 file)
1.34 KB,
patch
|
neil
:
review+
Callek
:
approval-comm-aurora+
Callek
:
approval-comm-beta+
Callek
:
approval-comm-release+
|
Details | Diff | Splinter Review |
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.
Comment 1•7 years ago
|
||
(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.
![]() |
Assignee | |
Comment 2•7 years ago
|
||
Hi Wladimir! Is there anything else to this bug? Otherwise I'm going to close this.
Flags: needinfo?(trev.moz)
Reporter | ||
Comment 3•7 years ago
|
||
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.
Flags: needinfo?(trev.moz)
![]() |
Assignee | |
Comment 4•7 years ago
|
||
(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.
![]() |
Assignee | |
Comment 5•7 years ago
|
||
> This code was then removed in bug 843800, so the hack is no longer necessary.
Updated•7 years ago
|
Attachment #8466986 -
Flags: review?(neil) → review+
![]() |
Assignee | |
Comment 6•7 years ago
|
||
comm-central trees are closed at the moment. But will push when it reopens.
status-seamonkey2.26:
--- → wontfix
status-seamonkey2.27:
--- → wontfix
status-seamonkey2.28:
--- → affected
status-seamonkey2.29:
--- → affected
status-seamonkey2.30:
--- → affected
status-seamonkey2.31:
--- → affected
![]() |
Assignee | |
Comment 7•7 years ago
|
||
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 [1] 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. [1] Bug 782516 [tabbrowser.xml] onLocationChange: aRequest can be null for an error page.
Attachment #8466986 -
Flags: approval-comm-release?
Attachment #8466986 -
Flags: approval-comm-beta?
Attachment #8466986 -
Flags: approval-comm-aurora?
Updated•7 years ago
|
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+
![]() |
Assignee | |
Comment 8•7 years ago
|
||
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
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-seamonkey2.32:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.32
You need to log in
before you can comment on or make changes to this bug.
Description
•