1. go to http://www.slunecnice.cz/ or http://musicmoz.org/ for first time
or reload it with Ctrl+Shift+R
Headers of both sides are rendered wrong, on right side are artefacts. They
disappear after fisrt reflow and page looks well, but first impression of
website (or Mozilla rendering) is very bad.
See attached screenshots.
Created attachment 124434 [details]
multimoz.org (wrong rendering)
All screenshots are from 2003052805/1.4branch/W2K
Created attachment 124435 [details]
multimoz.org (correct rendering)
Created attachment 124436 [details]
slunecnice.cz (wrong rendering)
Created attachment 124437 [details]
slunecnice.cz (correct rendering)
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4) Gecko/20030527
Saw the defects as shown in the attachments posted. Also notice problems with
the column widths in the middle of the http://musicmoz.org/ page. They changed
WFM on WinXP 2003052808, but I have seen it at other sites.
Confirming according comment #5.
With today branch build I was unable to reproduce on Slunecnice.cz, so I'm
clearing request to block 1.4.
But rendering is still broken with trunk build.
-> Layout: View Rendering
Might this have the same cause as bug 194638?
David: Probably yes, because rendering gets wrong after scrollbars appear.
Layout is centered and when scrollbars appear, the real center of viewport has
to move several pixels to left.
(So caused also by fixing bug 190193?)
Created attachment 127388 [details]
Created attachment 127497 [details] [diff] [review]
patch that allows the bug to be seen on Linux
This changes the Linux painting code so that the bug can be debugged on Linux.
Created attachment 127499 [details]
This one requires clicking, but clicking twice shows something that may be
I think the problem may be related to being missing an invalidation codepath
for the relevant case for when a block moves -- but we're getting most of the
invalidation thanks to the size change.
Note that if I change the 'width: 50%' in attachment 127499 [details] to 'width: 10em'
(i.e., from percentage width to fixed width), there's no invalidation at all,
and I need to move another window over part of the document to see that
something has moved.
While poking around, I noticed that nsBlockReflowContext::mBlockShouldInvalidate
is always PR_FALSE and should be removed.
Created attachment 127675 [details] [diff] [review]
This fixes it. I think it's the right thing.
I assume that the intent of passing PR_FALSE into aDamageDirtyArea in
nsBlockFrame::DirtyLine is to avoid unnecessary invalidations if the entire
block is going to be repainted anyway. This is not the case for a Resize reflow.
By the way, I've noticed (with paint flashing) that when we resize a browser
window, the entire window contents is repainted no matter what.
How about calling this |forceInvalidate| and getting rid of the other
|forceInvalidate|, and getting rid of |BRS_DAMAGECONSTRAINED| too, since it's
never set? (It was an old optimization for text inputs before we had reflow roots.)
And feel free to get rid of mBlockShouldInvalidateItself too. (It's obviously
broken if it were used, since the |Invalidate| calls use |mRect|, which is in
the wrong coordinate system.)
Created attachment 127683 [details] [diff] [review]
includes the cleanups suggested by dbaron. Also includes a few fixes for
warnings I'm heartily sick of.
Comment on attachment 127683 [details] [diff] [review]
> // Check whether this is an incremental reflow
>- PRBool incrementalReflow = aState.mReflowState.reason ==
>- eReflowReason_Incremental ||
>- aState.mReflowState.reason ==
>+ PRBool doInvalidates =
>+ aState.mReflowState.reason == eReflowReason_Incremental
>+ || aState.mReflowState.reason == eReflowReason_Dirty
>+ || aState.mReflowState.reason == eReflowReason_Resize;
Could you put the || at the end of the line to be consistent with the existing
> rv = ReflowLine(aState, line, &keepGoing, forceInvalidate);
and rename the variable where you use it, too?
>+ rv = ReflowLine(aState, line, &keepGoing, forceInvalidate);
Perhaps it would be better to use NS_STATIC_CAST(void*, ...) rather than
(void*) to be consistent with the casts already there?
Once you compile, and with those changes, r+sr=dbaron.
oops ... I decided to change the name to doInvalidate at the last minute.
forceInvalidate is a misnomer since the default behavior is with it set to
PR_TRUE; setting it to PR_FALSE is an optimization.
I'll do all you suggest, including using NS_STATIC_CAST. Thanks.
Works fine with 2003071508/trunk/W2K on both sites and testcase. Thanx!
mozilla1.5a released. unsetting request.