Closed Bug 288888 Opened 15 years ago Closed 15 years ago
Don't create scrollbars for scrolling="no" IFRAMEs (fix perf regression)
I have a hunch that the performance regression when creating a full-fledged nsHTMLScrollFrame for scrolling="no" IFRAMEs is because we create anonymous scrollbar content where we didn't before. Indeed, we are creating anonymous scrollbars for such IFRAMEs. You'd think the call to GetScrollbarStyles in CreateAnonymousContent would pick up the docshell's scrollbar preferences and return HIDDEN so that we don't create the scrollbars at all. Unfortunately in that call the scrollframe has not yet been added to the viewport's child list so when we check that the scrollframe is the first child of the viewport, it fails. Instead we can just record explicitly whether we're the viewport scrollframe and avoid peeking at the frame hierarchy, and we no longer create scrollbars for those IFRAMEs. I think this will fix the performance regression.
The big comment explains what's happening. Because HTML/BODY overflow propagation hasn't happened yet, the GetScrollbarStyles call is really only checking the container's scroll preferences (which is what we want, since we want dynamic changes to propagated overflow to work without reconstructing the viewport frame, otherwise we'd need more annoying detection/propagation logic). Dynamic changes to the scrolling attribute won't work, but then again, they never have.
checked in. It looks like this has indeed resolved the perf regression.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
roc, one of your checking has fixed a long-time bug when rendering http://www.shacknews.com/ On previous builds the text inside the 'latest interesting comments' rendered incorrectly until i changed font size. now it renders properly first time. just a fyi.
The fix for this couldn't be responsible for bug 289766, could it? The timing is right, as is the scrolling="no" requirement. Just checking.
I'd be surprised. This patch only changes whether we create scrollbars and should not have any effect on how content areas get painted.
Then again, I've been surprised before. Someone could verify by backing out this patch and seeing if it fixes the regression.
Is anyone able to create a build with the patch backed-out? I'm happy to test it to see if it caused the bug I mentioned in comment 4, but I have no way of managing a cvs on my Mac (including the know-how) to create the build myself. Thanks :)
You need to log in before you can comment on or make changes to this bug.