wrong rendering of websites (after Ctrl+Shift+R)

VERIFIED FIXED

Status

()

Core
Layout: View Rendering
VERIFIED FIXED
14 years ago
14 years ago

People

(Reporter: Adam Hauner, Assigned: roc)

Tracking

({testcase})

Trunk
x86
Windows 2000
testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(7 attachments, 2 obsolete attachments)

(Reporter)

Description

14 years ago
Repro:
1. go to http://www.slunecnice.cz/ or http://musicmoz.org/ for first time
   or reload it with Ctrl+Shift+R

Actual:
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.
(Reporter)

Comment 1

14 years ago
Created attachment 124434 [details]
multimoz.org (wrong rendering)

All screenshots are from 2003052805/1.4branch/W2K
(Reporter)

Comment 2

14 years ago
Created attachment 124435 [details]
multimoz.org (correct rendering)
(Reporter)

Comment 3

14 years ago
Created attachment 124436 [details]
slunecnice.cz (wrong rendering)
(Reporter)

Comment 4

14 years ago
Created attachment 124437 [details]
slunecnice.cz (correct rendering)

Comment 5

14 years ago
Confirming:
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
when refreshed.

Comment 6

14 years ago
WFM on WinXP 2003052808, but I have seen it at other sites.
(Reporter)

Comment 7

14 years ago
Confirming according comment #5.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.4?
Summary: wrong rendering of website (after Ctrl+Shift+R) → wrong rendering of websites (after Ctrl+Shift+R)
(Reporter)

Comment 8

14 years ago
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
Assignee: other → roc+moz
Component: Layout → Layout: View Rendering
Flags: blocking1.4?
(Reporter)

Updated

14 years ago
Flags: blocking1.5a?
Might this have the same cause as bug 194638?
Depends on: 194638
(Reporter)

Comment 10

14 years ago
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]
simple testcase
(Reporter)

Updated

14 years ago
Keywords: testcase
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]
simple testcase

This one requires clicking, but clicking twice shows something that may be
interesting.

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.
Attachment #127388 - Attachment is obsolete: true
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]
fix

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]
better fix

includes the cleanups suggested by dbaron. Also includes a few fixes for
warnings I'm heartily sick of.
Attachment #127675 - Attachment is obsolete: true
Comment on attachment 127683 [details] [diff] [review]
better fix

>   // Check whether this is an incremental reflow
>-  PRBool  incrementalReflow = aState.mReflowState.reason ==
>-                              eReflowReason_Incremental ||
>-                              aState.mReflowState.reason ==
>-                              eReflowReason_Dirty;
>+  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
style?

>       rv = ReflowLine(aState, line, &keepGoing, forceInvalidate);

and rename the variable where you use it, too?

>+      rv = ReflowLine(aState, line, &keepGoing, forceInvalidate);

here too?

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.
Attachment #127683 - Flags: superreview+
Attachment #127683 - Flags: review+
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.
fixed
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
(Reporter)

Comment 24

14 years ago
Works fine with 2003071508/trunk/W2K on both sites and testcase. Thanx!
v=aha
Status: RESOLVED → VERIFIED
Blocks: 195598
Blocks: 204289

Comment 25

14 years ago
mozilla1.5a released. unsetting request.
Flags: blocking1.5a?
You need to log in before you can comment on or make changes to this bug.