Version: (using KDE Devel) Installed from: Compiled sources Compiler: gcc 3.3.3 OS: Linux the following causes khtml to be unresponsive on even very fast hardware: <html> <body> <style type="text/css"> .dropShadow { position: relative; left: 20px; } .searchBoxHome { float: left; } </style> <div class="dropShadow"> <div class="searchBoxHome"> <form> <input type="text" value="Jitter back and forth!"> </form> Jitterbug </div> </div> </body> </html> due to the nested <div>'s, text content gets painted twice at an offset of 20px. for form elements, this triggers an infinite loop and the text field moves back and forth as quickly as possible (which, it turns out, is pretty quick =)
Created attachment 8780 [details] above html in a handy file
Example sites deviantart.com looks to expereince this with the username/password fields joeuser.com also looks to expereince this with the email/password fields, for me I have to click the 'decrease font size' button first in konq's toolbar
Created attachment 9117 [details] Possible Patch (one liner, like all the best ones :) Fixes this bug. It causes a few regressions, but I can't seem to tell if the regressions are actually any different, since this changes the tree build up slightly. I'm not experienced enough with khtml to know if I actually introduced any new regressions is what I'm saying :)
re #3: that's not the right fix I'm afraid. Floats don't need layers (unless they have some overflow clip). Still it gives interesting clues.
Germain, what does a Layer do in khtml exactly?
Created attachment 9240 [details] a new possible patch this patch fixes it. It fixes deviantart, various web sites I've been to (like eric meyer's) continue to render fine as well. the overlapping text goes away (which is a strongly related problem). 3 new PASSes in the regressions, 0 new failures (but I have suspicions that all of the regression tests are not done)
there's some old cruft in that patch (like int dx). never mind it, I'll remove if/when I commit.
re #5: Hi Charles, Layers are used for two purposes: to render an object that might be larger than the effectively displayed rectangle (thus having some overflow, which can be hidden, or accessible through scrollbars) - case of all objects having an overflow CSS property != visible, and to represent an object that is arbitrarily placed (positioned) and thus needs a precise stacking order for painting (painting is also clipped to the rectangle of the layer). About your new patch: it looks right on the culprit indeed, but as it seemingly boils down to "do not copy floats accross layer boundaries", that should be clearly stated... also I'm not sure if it is always true that floats can't overhang accross layer boundaries... need to think a bit about it... might be... but then what was the purpose of the original code? mmh.
I'll commit with some additional commenting tonight unless I hear otherwise. Thanks Germain! Take a look at: http://aseigo.blogspot.com/2005/01/roblimo-thinks-my-mom-is-hot.html -- particularly Aaron's picture to the right. My patch changes the behavior of the bugfix such that his face is partially obscured, instead of some text overlapping it. It helps to further expose another bug (but I don't think it's really a regression).
My patch causes this page: http://www.audioscrobbler.com/user/njaard/network/ to not put a linebreak between "Friends" and "Musical Neighbours" :(
mmh, yes, I read your patch backward last night: it doesn't stop copying accross layer boundaries, but rather within the same layer boundaries: (flow->enclosingLayer() == enclosingLayer()). That is definetly wrong. Are you sure your testregression setup is OK? How do you run it? How many test passed does it report at exit? The recent Xvfb changes make it a bit difficult to setup. Anyway, I'm having a look too... the noPaint flag logic looks flawed.
I know now for a fact that it's not set up. the font ahem consistantly crashes any client that tries to access it without XFT (xfontsel, even). That is even on my Real X-Server and not just Xvfb. I think something is wrong with that font.
Created attachment 9249 [details] the middle ground This seems to work, it brings aaron's page back to the old incorrect formatting, and audioscrobbler's page works as well. Its difference from my previous patch should be quite obvious.
My last patch doesn't fix deviantart, but it does fix the test case! :(
Created attachment 9254 [details] test case for deviantart this is a testcase for a bug that occurs with my latest patch at deviantart.
Created attachment 9255 [details] fixes deviantart this fixes deviantart, the testcase stops jittering as well, but you can't seem to type into the inputs for some reason. This is most clearly a hack
re #16: well, as the point is to avoid toggling once we have crossed layers, what about using a bool member instead? We already have a bitfield open in FloatingObject anyway, so that would be fine.
Created attachment 9257 [details] proposed patch
Yours seems to do close to what mine does, but it's faster, and for some reason doesn't have that weird "can't use lineedit" bug.
*** Bug 97851 has been marked as a duplicate of this bug. ***
OK. The regression output is fine too, so let's go ahead... BTW, did you got my answer to your 'clarify code a bit.' commit? I'll revert the s/break/return/ change in the same go because it's not equivalent (regressing e.g on webcore/fast/dynamic/002.html)
CVS commit by ggarand: float double-painting bug: prevent noPaint from being toggled off once we have crossed a layer during the float propagation process. Most of the work and investigation done by Charles Samuels. BUG: 95704 M +9 -0 ChangeLog 1.367 M +35 -29 rendering/render_block.cpp 1.59 M +2 -0 rendering/render_block.h 1.24 --- kdelibs/khtml/ChangeLog #1.366:1.367 @@ -1,2 +1,11 @@ +2005-01-25 Germain Garand <germain@ebooksfrance.org> + + fix #95704: float double-painting bug. Most of the work done by Charles Samuels. + + * rendering/render_block.h (FloatingObject::FloatingObject): add crossedLayer flag to prevent noPaint from + being toggled off once we have crossed a layer during the float propagation process + + * rendering/render_block.cpp (addOverHangingFloats): use/update said flag. + 2005-01-19 Allan Sandfeld Jensen <kde@carewolf.com> --- kdelibs/khtml/rendering/render_block.cpp #1.58:1.59 @@ -1079,5 +1079,5 @@ void RenderBlock::layoutBlockChildren( b prevFlow = static_cast<RenderBlock*>(child); - if (child->hasOverhangingFloats() && !child->style()->hidesOverflow()) { + if (child->hasOverhangingFloats() && !child->hasOverflowClip()) { // need to add the child's floats to our floating objects list, but not in the case where // overflow is auto/scroll @@ -1906,8 +1906,12 @@ void RenderBlock::addOverHangingFloats( if ( ( !child && r->endY > offset ) || ( child && flow->yPos() + r->endY > height() ) ) { - - if (child && (flow->enclosingLayer() == enclosingLayer())) + if (child && !r->crossedLayer) { + if (flow->enclosingLayer() == enclosingLayer()) { // Set noPaint to true only if we didn't cross layers. r->noPaint = true; + } else { + r->crossedLayer = true; + } + } FloatingObject* f = 0; @@ -1915,8 +1919,8 @@ void RenderBlock::addOverHangingFloats( QPtrListIterator<FloatingObject> it(*m_floatingObjects); while ( (f = it.current()) ) { - if (f->node == r->node) return; + if (f->node == r->node) break; ++it; } - + if ( !f ) { FloatingObject *floatingObj = new FloatingObject(r->type); floatingObj->startY = r->startY - offset; @@ -1934,7 +1938,8 @@ void RenderBlock::addOverHangingFloats( floatingObj->noPaint = true; } - else - // Only paint if |flow| isn't. - floatingObj->noPaint = !r->noPaint; + else { + floatingObj->noPaint = (r->crossedLayer || !r->noPaint); + floatingObj->crossedLayer = r->crossedLayer; + } floatingObj->width = r->width; @@ -1946,4 +1951,5 @@ void RenderBlock::addOverHangingFloats( } } + } } --- kdelibs/khtml/rendering/render_block.h #1.23:1.24 @@ -192,4 +192,5 @@ protected: width = 0; noPaint = false; + crossedLayer = false; } @@ -201,4 +202,5 @@ protected: Type type : 1; // left or right aligned bool noPaint : 1; + bool crossedLayer : 1; // lock noPaint flag };
#21: No I didn't, apologes. it's hard to tell on some of these functions the number of whiles you're in.
For what its worth, http://www.joeuser.com still jitters if I decrease the font size by 1 (I'm running 12/7 and Sans Serif in my font config / defaults).
#24: It looks to me like that's an unrelated bug related to the scrollbars.