Skip to content

Loading…

Candidate for fix for #1140 #1150

Merged
merged 8 commits into from

3 participants

@AlexVallat
Collaborator

As described in gorhill/uBlock#1140 (comment), this is a work around for Firefox sending document-element-inserted before nsIWebProgressListener.onLocationChange. It would, however, mean that we will usually get two location changes for the same page.

@AlexVallat AlexVallat Prevent resetting of block count statistics in the more normal case o…
…f onLocationChange occurring before document-element-inserted
f280c61
@AlexVallat
Collaborator

Yes, that's me, why?

@gorhill

Do you want to take over the add-on on Firefox?

@AlexVallat
Collaborator

Is @Deathamns leaving? That's bad news! I've just noticed you transferred the project to @chrisaljoudi too, wow.

OK, well, I'll certainly do my best to maintain the Firefox bits, although my time becomes more limited after the Easter holidays finish.

How do you want to handle the AMO listing - do you want me to maintain that, or is @chrisaljoudi already doing that as part of the build process?

@gorhill

do you want me to maintain that

That would be best, as you worked on the Firefox-specific code. What e-mail do I use to add you as a manager?

@AlexVallat
Collaborator

My AMO account email is -edit removed-, thanks

@chrisaljoudi: When you get a few minutes, we should talk about build and deployment processes, and see how much automation can be achieved. Also, what sort of dev release / blessed release system you want to use - once it's fully reviewed we can push out a faster release developer channel as well as reviewed full releases.

@AlexVallat
Collaborator

On reflection, this is not really the right place to discuss this, so I've created an issue for it: #1166

Returning briefly to topic - If @Deathamns is no longer participating, then I guess code review for this is up to @chrisaljoudi now, perhaps when things calm down a bit!

@AlexVallat AlexVallat referenced this pull request
Merged

Fixing #1149 #1195

@chrisaljoudi chrisaljoudi merged commit 738c695 into chrisaljoudi:master
@AlexVallat AlexVallat added a commit to AlexVallat/uBlock that referenced this pull request
@AlexVallat AlexVallat Cherry pick other part of fix for #1140 (#1150) beccbe6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 1, 2015
  1. @AlexVallat
Commits on Apr 2, 2015
  1. @AlexVallat

    Prevent resetting of block count statistics in the more normal case o…

    AlexVallat committed
    …f onLocationChange occurring before document-element-inserted
Commits on Apr 4, 2015
  1. Fix issue template link

    committed
  2. Missed one

    committed
  3. @AlexVallat
  4. @AlexVallat

    Prevent resetting of block count statistics in the more normal case o…

    AlexVallat committed
    …f onLocationChange occurring before document-element-inserted
  5. @AlexVallat
This page is out of date. Refresh to see the latest.
Showing with 19 additions and 5 deletions.
  1. +4 −5 CONTRIBUTING.md
  2. +9 −0 platform/firefox/frameModule.js
  3. +6 −0 platform/firefox/vapi-background.js
View
9 CONTRIBUTING.md
@@ -10,15 +10,14 @@ First of all, thank you for taking the time to help improve uBlock!
Probably the easiest way to submit an issue.
-**[Use this link to start with the standard issue template.](https://github.com/gorhill/uBlock/issues/new?title=[BrowserName]%20ShortDescription&body=%3C!--%0AInstructions%3A%0A%0AReplace%20the%20relevant%20parts%20of%20this%20template%20%0Awith%20details%20applicable%20to%20your%20case.%20Please%0Adon't%20remove%20the%20headers%2Fsubtitles.%0A%0ADon't%20worry%20about%20removing%20these%20instructions%3B%0Athey're%20not%20visible%20once%20you%20submit%20your%20issue.%0A%0AFor%20details%20about%20issues%2C%20check%20out%3A%0Ahttps%3A%2F%2Fgithub.com%2Fgorhill%2FuBlock%2Fblob%2Fmaster%2FCONTRIBUTING.md%23before-you-submit%0A--%3E%0A%0A%23%23%20Steps%20to%20Reproduce%0A1.%20Replace%20this%20example%20list%20with%20a%20list%20of%20steps%20to%20reproduce%20the%20issue%0A2.%20Example%20step%202%0A3.%20Feel%20free%20to%20add%20more%20steps%0A%0A%23%23%20Symptoms%0AReplace%20this%20with%20a%20description%20of%20what%20the%20symptoms%20you're%20observing%20are.%0A%0A%23%23%20Preferences%20Different%20From%20Defaults%0AWe%20recommend%20a%20screenshot%20—%20include%20any%20filter%20lists%20you%20enabled%2Fdisabled%2C%20whitelisted%20sites%2C%20etc.%0A%0A%23%23%20Info%0A%0A%60%60%60%0AuBlock%20version%3A%0A%20%20%20%200.0.0.0%0ABrowser%20and%20version%3A%0A%20%20%20%20Browser%201.2.3%0AOS%20and%20version%3A%0A%20%20%20%20OS%2010%0A%60%60%60%0A%0A%23%23%20Other%20Extensions%0A%0A*%20None.)**
+**[Use this link to start with the standard issue template.](https://github.com/chrisaljoudi/uBlock/issues/new?title=[BrowserName]%20ShortDescription&body=%3C!--%0AInstructions%3A%0A%0AReplace%20the%20relevant%20parts%20of%20this%20template%20%0Awith%20details%20applicable%20to%20your%20case.%20Please%0Adon't%20remove%20the%20headers%2Fsubtitles.%0A%0ADon't%20worry%20about%20removing%20these%20instructions%3B%0Athey're%20not%20visible%20once%20you%20submit%20your%20issue.%0A%0AFor%20details%20about%20issues%2C%20check%20out%3A%0Ahttps%3A%2F%2Fgithub.com%2Fchrisaljoudi%2FuBlock%2Fblob%2Fmaster%2FCONTRIBUTING.md%23before-you-submit%0A--%3E%0A%0A%23%23%20Steps%20to%20Reproduce%0A1.%20Replace%20this%20example%20list%20with%20a%20list%20of%20steps%20to%20reproduce%20the%20issue%0A2.%20Example%20step%202%0A3.%20Feel%20free%20to%20add%20more%20steps%0A%0A%23%23%20Symptoms%0AReplace%20this%20with%20a%20description%20of%20what%20the%20symptoms%20you're%20observing%20are.%0A%0A%23%23%20Preferences%20Different%20From%20Defaults%0AWe%20recommend%20a%20screenshot%20—%20include%20any%20filter%20lists%20you%20enabled%2Fdisabled%2C%20whitelisted%20sites%2C%20etc.%0A%0A%23%23%20Info%0A%0A%60%60%60%0AuBlock%20version%3A%0A%20%20%20%200.0.0.0%0ABrowser%20and%20version%3A%0A%20%20%20%20Browser%201.2.3%0AOS%20and%20version%3A%0A%20%20%20%20OS%2010%0A%60%60%60%0A%0A%23%23%20Other%20Extensions%0A%0A*%20None.)**
---
### Before you submit
-1. Submit bugs/issues only.
-1. Do **not** submit design ideas: any such issue will be closed without comment.
-1. Make sure your issue [hasn't already been fixed in a recent release](https://github.com/gorhill/uBlock/releases). That's good news!
+1. Please submit bugs/issues only.
+1. Make sure your issue [hasn't already been fixed in a recent release](https://github.com/chrisaljoudi/uBlock/releases). That's good news!
1. Verify that the issue does **not** occur with uBlock disabled. If it still occurs with uBlock disabled, it's probably not an issue with uBlock.
---
@@ -36,7 +35,7 @@ To help us diagnose and fix the problem, please always, always include the follo
* A screenshot of **any** of uBlock's preferences that differ from the defaults
* This includes a whitelisted website, enabled/disabled filter list, anything
* Please do include everything different from the defaults whether or not it seems relevant to your issue
-* The version of uBlock you're having the issue with; you can find this in [uBlock's popup UI](https://github.com/gorhill/uBlock/wiki/Quick-guide:-popup-user-interface)
+* The version of uBlock you're having the issue with; you can find this in [uBlock's popup UI](https://github.com/chrisaljoudi/uBlock/wiki/Quick-guide:-popup-user-interface)
* Example: `uBlock 0.9.0.0`
* The browser you're using and its version
* Examples: `Firefox 36`, `Safari 8.0.5`, `Chrome 41.0.2272`
View
9 platform/firefox/frameModule.js
@@ -293,6 +293,15 @@ const contentObserver = {
let docReady = (e) => {
let doc = e.target;
doc.removeEventListener(e.type, docReady, true);
+
+ // It is possible, in some cases (#1140) for document-element-inserted to occur *before* nsIWebProgressListener.onLocationChange, so ensure that the URL is correct before continuing
+ let messageManager = doc.docShell.getInterface(Ci.nsIContentFrameMessageManager);
+
+ messageManager.sendSyncMessage(locationChangedMessageName, {
+ url: loc.href,
+ noRefresh: true, // If the URL is the same, then don't refresh it so that if this occurs after onLocationChange, no the block count isn't reset
+ });
+
lss(this.contentBaseURI + 'contentscript-end.js', sandbox);
if ( doc.querySelector('a[href^="abp:"]') ) {
View
6 platform/firefox/vapi-background.js
@@ -1325,6 +1325,12 @@ vAPI.net.registerListeners = function() {
var locationChangedListener = function(e) {
var details = e.data;
var browser = e.target;
+
+ if (details.noRefresh && details.url === browser.currentURI.asciiSpec) { // If the location changed message specified not to refresh, and the URL is the same, no need to do anything
+ //console.debug("nsIWebProgressListener: ignoring onLocationChange: " + details.url);
+ return;
+ }
+
var tabId = vAPI.tabs.getTabId(browser);
if (tabId === vAPI.noTabId) {
return; // Do not navigate for behind the scenes
Something went wrong with that request. Please try again.