Closed Bug 337356 Opened 14 years ago Closed 14 years ago
Page zoom breaks if content policy accesses frame's document
This issue has been reported for Adblock Plus 0.7 - if you zoom a page with a frame and then reload it the frame will show text in normal size while the containing page is still zoomed. Attaching minimized test case.
This content policy does only one thing - it accesses node.contentWindow.document for frame nodes, that's enough to break page zoom. Simply save it as TestPolicy.js and drop it into the application's components directory. Then remove compreg.dat in your profile to make the component register.
Forgot to mention: the testcase won't work with Adblock Plus 0.7 because it uses a data: URL that will be ignored.
Yep. Zoom is gotten from the previous content viewer, or from the parent if there isn't one. You're forcing content viewer creation when you try to get the document from a window that doesn't have one yet, and CreateAboutBlankContentViewer doesn't copy zoom state, along with a bunch of other things.
Actually, I was wrong. CreateAboutBlankContentViewer does copy state. But SetTextZoom and GetTextZoom on DocumentViewerImpl are broken -- they only work if there is an mPresContext, which is not going to be the case for the about:blank viewer here. So the viewer should probably store the text zoom in a member, and possibly set it on the prescontext when creating it?
So something like this?
That looks about right, yes. Does it work?
Yes, it seems to work. I'll recheck with backing out the patch.
Comment on attachment 222355 [details] [diff] [review] patch Yes, backing out the patch makes the bug appear again.
Comment on attachment 222355 [details] [diff] [review] patch Add an assert in GetTextZoom that !mPresContext || mPresContext->TextZoom() == mTextZoom, and r+sr=bzbarsky
Assignee: general → martijn.martijn
Boris, something like this?
Attachment #222355 - Attachment is obsolete: true
Er, like this.
Attachment #223807 - Attachment is obsolete: true
Comment on attachment 223839 [details] [diff] [review] patch2, with assertion You want: + NS_ASSERTION(!mPresContext || mPresContext->TextZoom() == mTextZoom, + "mPresContext->TextZoom() != mTextZoom");
Attachment #223839 - Flags: review-
Yeah, sorry about continuously wrong patches here. I'll do what you say in comment 13 and will check that in (hopefully) tomorrow.
Checked that patch in. Sorry for the delay. Checking in layout/base/nsDocumentViewer.cpp; /cvsroot/mozilla/layout/base/nsDocumentViewer.cpp,v <-- nsDocumentViewer.cpp new revision: 1.490; previous revision: 1.489 done
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.