Closed Bug 419798 Opened 16 years ago Closed 16 years ago

Adblock Plus broken on trunk

Categories

(Firefox :: Extension Compatibility, defect, P1)

Product:
Firefox
Component:
Extension Compatibility
Platform:
x86
macOS
Type:
defect

Tracking

()

Status:
RESOLVED INVALID
Milestone:
Firefox 3 beta4

People

(Reporter: Dolske, Assigned: smaug)

References

Details

(Keywords: relnote, Whiteboard: [waiting on new ABP release]) 

After updating my trunk tree today, I found that Adblock Plus stopped working. It wasn't blocking ads, the toolbar button disappeared, and I got gobs of console errors like:

this.boxObject is null" {file: "chrome://adblockplus/content/sidebar.js" line:
711}]' when calling method: [nsIContentPolicy::shouldLoad]"

Backing out the recent checkin for bug 402982 fixed the problem. Not sure if the problem is with that patch, or if Adblock Plus is doing something wrong that just happened to work before. In any case, it's a popular extension so someone needs to fix something. :-)
Flags: blocking-firefox3?
Priority: -- → P1
P1 until we can figure out what the issue is.

Wladimir?
Flags: blocking-firefox3? → blocking-firefox3+
Minefield build 2008022604 is working fine, bug 402982 landed too late for this nightly. I will update my tree but that will take a while.

Unfortunately, I don't have access to bug 402982. Looking at the patch, it might have broken assumptions Adblock Plus makes about initialization of frames - which is probably the fault of Adblock Plus (this doesn't make fixing the issue any easier, attaching data to frames was always the most problematic part of the whole extension).
I gave you access to bug 402982.  Hope it helps, or at least saves you some time understanding the change.
Thank you, Jesse. Unfortunately, I don't really see how bug 402982 would affect Adblock Plus. In my Windows build it doesn't. I will wait for the nightly to be sure but the problem seems to be OS-specific - and I don't currently have a working OS X emulator.
I installed adblock plus and I do have the toolbar button etc, and adblock
did add those small "buttons" to web content adds.
Couldn't see any chrome://adblockplus/content/sidebar.js errors.
This is on linux.
Justin, what are the steps-to-reproduce?
I don't see any issues with 2008022704 nightly on Windows XP either.
Justin, are you really sure this is a regression from bug 402982?
I think the right bug is bug 395609, which was backed out - at least with
that patch I can reproduce the js error.
And if bug 395609 causes this, it is probably because with that <xul:browser> 
loads the document when it is added to document, not when the element is set
visible.
Hmm, so, not sure what happened here.

I had first backed out 395609 locally, since it was the most recent suspicious looking patch (and was easier to |patch -R| since it didn't have an associated bustage fix. :). That build was still broken. Then I backed out 402982 (cvs reverted the bustage fix, then reverted the patch), and that build worked. I then relanded 395609 and rebuilt, just to be sure, and that was still working.

However, I can't reproduce the Adblock bustage with a current trunk nightly. My debug build is also working now (sync'd with CVS just after midnight, no local diffs/backout).

So, I can only assume that either I fubared something in my regression hunting, or that whatever broke was fixed in some other way since then. Probably the former, since it sounds like you can reproduce this by relanding 395609? :(

I'm inclined to WFM this, since Adblock is currently working and it's hard to troubleshoot builds that don't exist any more. I'll add a note to 395609 about this issue. I guess reopen this if whatever 395609 is doing is correct, would break Adblock again when it relands?
Blocks: 395609
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → WORKSFORME
Vladimir, would it be bad if xul:browser page loading was changed to start
when element is bound to document (not when the element gets visible)?
Using Adblock plus with that change has the problems this bugs talks about and
I wonder how big the changes would be to fix ABP.
I have no idea why that would affect Adblock Plus - I will need to try applying that patch to my tree.
Wladimir, could you try these builds https://build.mozilla.org/tryserver-builds/2008-03-04_05:50-opettay@mozilla.com-1204638534/ ?
Those builds have the patch which make xul frame (browser/iframe/editor) loading
work like html:iframe.
This breaks Adblock Plus indeed - but only the list of blockable items, everything else is fine. This list is built similar to a sidebar (was originally a sidebar), there is an <iframe> loading the XUL document with the actual list. But unlike for a sidebar the "src" attribute of this <iframe> never changes, I simply assumed that the document will load when the frame becomes visible and unload when it is hidden.

This assumption is no longer true with the patch from bug 395609. The document containing the list is always loaded and registers an observer at the Adblock Plus component to get called whenever something is blocked. Problem is, it tries to update the list but accessing the <tree> element fails - it is invisible and its XBL binding didn't apply.

I just did the change to set the "src" attribute only when the list becomes visible and to unset it whenever it is hidden. With that change, everything works fine again.
I checked in the patch for bug 395609, so I guess ABP needs to be changed now
(unless it was changed already).
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
I did that change already - now I only need to find time to do a release.
So is this bug FIXED, then?
I can resolve it when there is actually an Adblock Plus version available that considers the new behavior. Or you can just resolve it now because I don't think Mozilla is going to do anything about it.
Wladimir, if you need any help, ping me on IRC.

I think it is better to keep this open, so that whoever uses nightly builds+ABP
can easily find this.
Whiteboard: [waiting on new ABP release]
A development build that works properly with current nightlies: http://adblockplus.org/development-builds/more-firefox-3-fixes
Respectfully marking INVALID, as there's no trunk issue here.

Wladimir, please comment in this bug once you've released a build that works with nightlies. Until then we'll have a relnote ready for this.
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Keywords: relnote
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.