Closed Bug 77507 Opened 24 years ago Closed 24 years ago

flashing background in random color

Categories

(Core Graveyard :: GFX, defect, P2)

Product:
Core Graveyard
Component:
GFX
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.2

People

(Reporter: db, Assigned: roc)

References

(
URL
)

Details

Attachments

(8 files)

sample of unwanted color painting. current CVS build linux
57.83 KB, image/jpeg
Details
Set default background color for all view manager instances
1.79 KB, patch
Details | Diff | Splinter Review
Looking-good patch
5.94 KB, patch
Details | Diff | Splinter Review
Patch v2 (added warning to nsFrame::SetDefaultBackgroundColor)
6.67 KB, patch
Details | Diff | Splinter Review
patch v3 - remove inadvertent #ifdef DEBUG_roc hunks
6.04 KB, patch
Details | Diff | Splinter Review
Verification diff
6.98 KB, patch
Details | Diff | Splinter Review
Changes to Windows widget layer to fix colors
1.20 KB, patch
Details | Diff | Splinter Review
Revised patch (v4); updated to handle hyatt's changes to nsDocShell
6.75 KB, patch
Details | Diff | Splinter Review 
When loading pages mozilla draws the background of each frame in some random
color before it is painted over with the correct content of the page.

Why paint it in a random color first when it is beeing redrawn anyway and then
even if the painting is not need, it's still strange to use bright colors likte
this when one could use some default background color instead. It doesn't flash
as much. Seems to happen on all pages, but for example the URL above shows this.

It think this drawing should go altogether since it's just a waste.

The query system doesn't seem to work so I could not search for the this bug.
But it's something that have been introduced between 2001041721 and mozilla
2001042408 (both linux nightly).
a result of bug 75591 being fixed?
Assignee: karnaze → kmcclusk
Component: Layout → Compositor
Seeing with PC Linux 2001042406.
Can't see this with Build ID 2001041212 / Win2000

I'm seeing this with a fresh CVS build, Linux.
In particular annoying in mailnews. For each newsgroup i select, the first
message read \will first show entire message/header area in either intense
purple, bright green or black :) None of these colors seem to have anything to
do with gtk theme color.

Confirming bug.
Status: UNCONFIRMED → NEW
Ever confirmed: true
I can't reproduce this at the given URL on Linux with CVS build from this 
morning. When I switch pages in the frame, the background is always drawn with 
the same background color as the last page.

In news, I don't see this either. When I switch to a new newsgroup, the message 
area is drawn in grey. When I click on a message, it displays. No strange colors 
appear.
Strange. I see it in sidebar too. For instance CNN expanded sidebar, which takes
a little while to load.
Closing it by clicking the grippy and then reopening causes the same  bug - a
random color covering whole area before content displays. Most often bright
green or black, but it seems to cycle through a whole reportoir - a few pale
ones too.

Saw this first time yesterday. Most often the colors are very intense. A fine
"psychedelic" bug when it strikes.

RH6.2 + all upgrades, XFree86 4.0.2 at Truecolor (16bit) nvidia, gtk+1.2.9,
gnome 1.2.4 with default theme, Sawfish.
I've been seeing this on slow pages like http://www.news.com

XFree 3.3.6, 16bpp
I saw that problem too in Win32 build 2001042420.

But I can only see dark grey color, not those bright random ones...

I've checked the Background Color setting in the Preference menu and I have not
defined the background color as that dark grey color.
A place to reproduce this easily:

http://www.hixie.ch/tests/evil/page-loading/DOM/001.cgi

click on the link.  Wait a bit before clicking on the alert.  The color you get
seems to depend on the length of the wait, or at least short waits always give
black.  So wait about two seconds.

Reproduces the problem every time in 2001-04-25-08 Linux build
Boris, where is this color supposed to show up? In the alert, or in the content 
area? I never see black anywhere and the content area is always the same.

The alert is painted in dark grey just before it goes away. Is that the bug 
you're referring to?
Arrgh. OK, I'm definitely not seeing that.
url from bug 77716 is another sample where i see rapid green/white flashing
going on "forever": http://www.svd.se/
I have color pref set to white bg, (and not "use system colors)
I have also checked the pref to "always use colors and background specified by
the web page"

It's like moz forgets my pref and instead make wild guesses inbetween loads in
document areas.
Not seeing it at all. Can you give me a sequence of page loads, starting with 
launching Mozilla, which produces the effect?
Robert, could you try a nightly?  I'm not seeing it in my CVS builds either.  
Also, does your X server have the XIE extension?  That may be relevant -- all the 
ones I have been testing on do.

The sequence for me is:
1) Start mozilla with http://www.mozilla.org as homepage
2) Go to http://www.hixie.ch/tests/evil/page-loading/DOM/001.cgi
3) Click the link
4) Wait 2 seconds before dismissing the alert

Oh, and I'm seeing this in the content area.

OK I think I can reproduce this now. I suspect it's simply an uninitialized 
value somewhere, which would explain the random colors and the varying 
difficulty of reproduction.
Found it. The problem is that at least with Win32 and GTK, 
nsIWidget::GetBackgroundColor is completely worthless (using the default 
implementation in nsBaseWidget). Unless a background color has been explicitly 
set on the widget, it returns an uninitialized value.

Therefore the fallback code path in nsViewManager::DefaultRefresh is producing a 
random background color to draw with. Yum.
But the question is why it paints the background at all since it's being
overpainted anyway? Of all the pages I have seen the background is never visible
when the page have been rendered with one of these random colors, that indicates
that this whole backround filling really is the bug, not just the color it
happens to be?
db:
That's not the bug as seen from here. After a showing erronous colors for a
brief while, the real content paints OK. And the feature of a color now being
initially set at all seems to fix that "null-content" i used to see when
focusing to another window and back while a page was loading. That used to cause
parts of another window to "glue" to content area - till what it was loading
actually started painting.

I wonder if you are seeing the color-bug in combo with another recent bug: bug
77717 being a sample. (There's more of those - 77717 is a dup i think)
Can you try force a repaint - by minimizing/resizing or otherwise? Are the wrong
colors still the only thing displaying? Or are they still overruling the page's
own background color?
R.K. Aa is right. This background color is only being used during the brief 
period between tearing down an old document and finding out what the new 
document's background is.
The patch makes us do something rational by default. The NS_WARNING is not being
triggered by my testing.

Stealing bug.
Assignee: kmcclusk → roc+moz
OS: Linux → All
Priority: -- → P2
Hardware: PC → All
Target Milestone: --- → mozilla0.9.1
Status: NEW → ASSIGNED
hyatt might enjoy being cc'd on this one.

/be
Nobody wants to nominate this nice little gem for M0.9?
Nominating... as a finishing touch to bug 75591 which was checked in M0.9.
Whiteboard: m0.9
Something is very wrong here. The nightly build I pulled is much much worse than 
my CVS builds. It has evil flashing on every pref pane switch.
And your pacth fixes the random colors, right?  [not counting the multiple
re-paints which is another problem of its own...]
This patch here fixes the random colors. It will paint the preferred background 
color instead.

But the disturbing part is that the fix to 75591 was supposed to make sure that 
(e.g.) in the prefs dialog, switching between panels would always show the theme 
background color as the background color, because the background color of the 
old panel would be propagated to the new panel before it started loading. That 
doesn't seem to be happening consistently in the nightly build and I don't know 
why.
is 0,0,0 black?  wouldn't white be better in most cases?
That color is never used. The PresContext always gives us a valid color.
It might perhaps be best then not to initialize the variable -- it adds to
confusion, and takes some cycles -- albeit insignificant.
It won't be used in practice, but it might be used in principle if the call to 
PresContext::GetDefaultBackgroundColor were changed to be able to fail.
In the spirit of bug 26739:

I liked this misbehavior (the random psychodelic colors flashing before loading
a new page). Please keep it as a pref =)

(and of course, make it more random; magenta is way too common here)
Turns out that, at least in the prefs pane, the document element's background
was being marked transparent. The real background was being painted by the root
box frame. So I moved the background-color-setting code to nsRootBoxFrame and
things are looking good. I thought I tried this before and it hadn't worked, but
I guess I made some other mistake.

This patch also incorporates the previous patch, so the background color is
always at least set to the default window background (no more psychodelic
colors). I also changed things so that if, for some reason, the root box frame
or the canvas frame do not have a background set, we leave the current
background color as is.
With 76495, you don't really need to do any of this.  I actually removed the 
code from docshell in my 76495 patch that set a background color.

Even once 76495 is checked in, I think we still want this because we can use the
background color information in other places (e.g., we can tell the underlying
platform about it to make resizing look nicer). It might also be deemed
necessary for 0.9. I assume you don't have any actual objections?
Yeah, the flashing makes things look unprofessional. Nice that roc could nail
this in the m0.9 timeframe. 

waterson who gave r= for bug 75591 is already cc:d; cc:ing attinasi who gave sr=
Nope, no objections.

OK, I'm targeting this at 0.9. Reviews from Marc, Chris W.?

Also, can someone test this in a 0.9 tree? I don't have one.
Target Milestone: mozilla0.9.1 → mozilla0.9
r=waterson
Pending testing in a 0.9 build, sr=attinasi
Fortunately, my m0.9 tree was still hanging around. I typed 'patch < diff.txt'
and 'nmake /f client.mak' -- no big deal so far, all is going well. I will be
reporting further details as the build completes.
Wow, it works like a charm. No flashing, no unexpected transient color.
Could we have this on the trunk too? Thanks!

You can still sense how the world is falling apart, but because the background
that is left there *is* the existing background on the current page, it just
gives the impression that: clicking wipes the text content. For an untrained
eye this makes things looks natural because the mind quickly associates 'click
to next page' with a smooth 'wipe current content'. While awaiting further
improvements from bug 76495, it would be nice to land this one on the trunk
too, even if is just for 'correctness' of the mBackgroundColor data field...

PS: I am hitting the NS_WARNING(). E.g., when I select the Prefs -> Fonts dialog
or when I go to http://www.hixie.ch/tests/evil/page-loading/DOM/001.cgi, or when
I navigate back/forwad from this page to the page containing the attachment of
the patch (haven't tested elsewhere as I want to finish filing this comment).
Here is the console output:
====

Document http://bugzilla.mozilla.org/ loaded successfully
WEBSHELL+ = 4
Disabling View Source StyleSheet
WEBSHELL+ = 5
Disabling View Source StyleSheet
Disabling View Source StyleSheet
/content/pref/pref-navigator.xul
WARNING: nsViewManager: Asked to paint a default background, but no default back
ground color is set!, file C:\Mozilla\src-m0.9\mozilla\view\src\nsViewManager.cp
p, line 967
Disabling View Source StyleSheet
/content/pref/pref-fonts.xul
WARNING: nsViewManager: Asked to paint a default background, but no default back
ground color is set!, file C:\Mozilla\src-m0.9\mozilla\view\src\nsViewManager.cp
p, line 967
Disabling View Source StyleSheet
/content/pref/pref-themes.xul
WARNING: nsViewManager: Asked to paint a default background, but no default back
ground color is set!, file C:\Mozilla\src-m0.9\mozilla\view\src\nsViewManager.cp
p, line 967
Disabling View Source StyleSheet
/content/pref/pref-colors.xul
WARNING: nsViewManager: Asked to paint a default background, but no default back
ground color is set!, file C:\Mozilla\src-m0.9\mozilla\view\src\nsViewManager.cp
p, line 967
Disabling View Source StyleSheet
/content/pref/pref-navigator.xul
WARNING: nsViewManager: Asked to paint a default background, but no default back
ground color is set!, file C:\Mozilla\src-m0.9\mozilla\view\src\nsViewManager.cp
p, line 967
Disabling View Source StyleSheet
/content/pref/pref-advanced.xul
WARNING: nsViewManager: Asked to paint a default background, but no default back
ground color is set!, file C:\Mozilla\src-m0.9\mozilla\view\src\nsViewManager.cp
p, line 967
Disabling View Source StyleSheet
/content/pref/pref-fonts.xul
WARNING: nsViewManager: Asked to paint a default background, but no default back
ground color is set!, file C:\Mozilla\src-m0.9\mozilla\view\src\nsViewManager.cp
p, line 967
/content/pref/pref-colors.xul
/content/pref/pref.xul
WEBSHELL- = 4
WEBSHELL- = 3
Disabling View Source StyleSheet
/
Enabling Quirk StyleSheet
Document http://bugzilla.mozilla.org/show_bug.cgi?id=77507 loaded successfully
Disabling View Source StyleSheet
/show_bug.cgi?id=77507
Enabling Quirk StyleSheet
Document http://bugzilla.mozilla.org/showattachment.cgi?attach_id=32683 loaded s
uccessfully
Disabling View Source StyleSheet
/showattachment.cgi?attach_id=32683
Enabling Quirk StyleSheet
WARNING: nsViewManager: Asked to paint a default background, but no default back
ground color is set!, file C:\Mozilla\src-m0.9\mozilla\view\src\nsViewManager.cp
p, line 967
Document http://bugzilla.mozilla.org/show_bug.cgi?id=77507 loaded successfully
Disabling View Source StyleSheet
/show_bug.cgi?id=77507
Enabling Quirk StyleSheet
Document http://bugzilla.mozilla.org/showattachment.cgi?attach_id=32683 loaded s
uccessfully
Disabling View Source StyleSheet
/showattachment.cgi?attach_id=32683
Enabling Quirk StyleSheet
WARNING: nsViewManager: Asked to paint a default background, but no default back
ground color is set!, file C:\Mozilla\src-m0.9\mozilla\view\src\nsViewManager.cp
p, line 967
Document http://bugzilla.mozilla.org/show_bug.cgi?id=77507 loaded successfully
Disabling View Source StyleSheet
/show_bug.cgi?id=77507
Enabling Quirk StyleSheet
Enabling Quirk StyleSheet
Document http://bugzilla.mozilla.org/ loaded successfully
Disabling View Source StyleSheet
/
Enabling Quirk StyleSheet
Document http://bugzilla.mozilla.org/show_bug.cgi?id=77507 loaded successfully
Disabling View Source StyleSheet
/show_bug.cgi?id=77507
Enabling Quirk StyleSheet
Document http://bugzilla.mozilla.org/showattachment.cgi?attach_id=32683 loaded s
uccessfully
Disabling View Source StyleSheet
/showattachment.cgi?attach_id=32683
Enabling Quirk StyleSheet
WARNING: nsViewManager: Asked to paint a default background, but no default back
ground color is set!, file C:\Mozilla\src-m0.9\mozilla\view\src\nsViewManager.cp
p, line 967
Document http://bugzilla.mozilla.org/show_bug.cgi?id=77507 loaded successfully


You should not be hitting the warning. Can you check that all the patch hunks 
were applied? At the least, the changes to nsFrame.cpp, nsDocShell.cpp and 
nsDocumentViewer.cpp should prevent the view manager warning from firing, 
because those changes ensure that a) all view managers have an initial default 
background color set and b) that color is never reset to any transparent color.

And what theme and platform are you using?
I'm not seeing those assertions in my 0.9 build.  It seems to work fine here.
Blizzard, you're on Linux I presume. I think Roger is on Windows. It could be a 
platform issue; maybe Windows forces a synchronous paint at some point before 
the background color is set by nsDocumentViewer.

    
    

    
  
I attached a dump of my console when verifying that all of the patch was applied 
correctly.

    
    

    
  
I was using Classic - Win2K. I just tried Modern, but I am still seing the
warnings.

    
    

    
  
Well, I'm stuck unless Roger can figure out what's going on in his build.

You could add debugging printfs to nsViewManager::Get/SetDefaultBackgroundColor 
(like I had at the end of 
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=32653) and see if that 
helps you see what's going on.

    
    

    
  
Looks like these are bogus warnings? I put back your printfs, and then
I changed:
====================

//      NS_WARNING("nsViewManager: Asked to paint a default background, but no 
default background color is set!");

to:
====================

       printf("WARNING: nsViewManager:%p Asked to paint a default background, 
but no default background color is set!\n", this);


And this is what I see (clearly, the setter of the bkg is called on
the _same_ viewmanager which is reporting that no bkg was set):
====================

BKG: Setting default background color on 3d0f480 from 0 to ffffff
WARNING: nsViewManager:03D0F480 Asked to paint a default background, but no defa
ult background color is set!
BKG: Setting default background color on 3d0f480 from ffffff to ffffffff
Document http://www.mozilla.org/ loaded successfully
WEBSHELL+ = 4
Disabling View Source StyleSheet
BKG: Setting default background color on 45bdb00 from 0 to ffffff
WEBSHELL+ = 5
Disabling View Source StyleSheet
BKG: Setting default background color on 4c18d60 from 0 to ffffff
Warning prev sibling is not in our list!!!Disabling View Source StyleSheet
BKG: Getting default background color from 4c18d60 = ffffff
/content/pref/pref-navigator.xul
BKG: Setting default background color on 4cc3a30 from 0 to ffffff
WARNING: nsViewManager:04CC3A30 Asked to paint a default background, but no defa
ult background color is set!

    
    

    
  
I assume you have your background preferences set to "platform defaults". Then 
it looks like the problem is that the Windows nsILookAndFeel implementation is 
buggy and returning colors which have a zero alpha channel.

http://lxr.mozilla.org/seamonkey/source/widget/src/windows/nsLookAndFeel.cpp#199

This code is wrong. Fix coming.

    
    

    
  
I also tried this, and the warnings went away:

NS_IMETHODIMP
nsViewManager::SetDefaultBackgroundColor(nscolor aColor)
{

+  aColor = NS_RGBA(NS_GET_R(aColor), NS_GET_G(aColor), NS_GET_B(aColor), 0xFF);

//#ifdef DEBUG_roc
    if (mDefaultBackgroundColor != aColor) {
      printf("BKG: Setting default background color on %x from %x to %x\n", 
this, mDefaultBackgroundColor, aColor);
    }
//#endif
    mDefaultBackgroundColor = aColor;
    return NS_OK;
}


    
    

    
  
Applying your patch now, will report what happens...

    
    

    
  
I presume that the patch still actually works with your fix?

Then I propose we take your nsViewManager fix (lower risk but ultimately wrong,
because we should be able to turn the background color off if we dare) for 0.9
and my fix for the trunk.

Roger, can you check in to the 0.9 branch? I think we have a= from blizzard. You
should be clear to check in to the branch.

    
    

    
  
I removed my oneliner fix, and applied your l&f patch. All is going well now, no 
more warnings on the console. Neither with Classic nor with Modern.

    
    

    
  
I think we should go with your change for 0.9 because it's lower risk.

I have to go offline for a while. Let's do this: let's ask waterson and attinasi 
to give the OK to your (additional) change, and then you check the whole lot 
(excluding for my look and feel patch) into the 0.9 branch. I will check into 
the trunk later (after getting review for the L&F change).

Later...

    
    

    
  
OK, though roc is confident that his l&f patch is the correct and long lasting
fix, he suggests to bake it a little bit on the trunk.

attinasi, waterson, any comments? are you agreable that I go ahead with my 
(temporary) emergency fix and checkin on the m0.9 branch on roc's behalf?



    
    

    
  
Updated my m0.9.1 tree to the tip with hyatt fix for bug 76495. That is a
different dimension. No more empty background of whatever color between clicks,
let alone a flashing or transient one. This bug has become academic on the
trunk... Seems that the bits to get and set the correct background is at most what
would be needed there. Things that can be optimized away from this bug would be
welcome.


    
    

    
  
We might as well get it into the trunk, anyway.  As for the branch, can we get
this in anyway?  We don't have a lot of baking time left there.

    
    

    
  
Ok, landing then. I was waiting for your go ahead signal.

    
    

    
  
Landed on the m0.9 branch. Retargetting to m0.9.1 to get if off the radars for 
the remaining landing that roc will made on the trunk.
Whiteboard: m0.9
Target Milestone: mozilla0.9 → mozilla0.9.1

    
    

    
  
roc, how is this one coming along?

    
    

    
  
BTW, on my Win2K, Debug -> XBL Demos are examples that still exhibit the 
transient background (I don't see this with m0.9).

    
    

    
  
Didn't this land on the trunk, too?

    
    

    
  
No, roc wanted a more elaborate fix on the trunk, rather than the one-liner that 
went in the m0.9 branch.

    
    

    
  
OK.  roc, what's the status here?
Whiteboard: critical for mozilla 0.9.1

    
    

    
  
I still plan to land this on the trunk. I'll try to do it tomorrow.

    
    

    
  
Something to keep in perspective is hyatt's patch in bug 76495 -attachment 32502 [details] [diff] [review]
which removed the code that was maintaining the value of the background in sync.

    
    

    
  
Yeah. Suck. I guess I have to revise the patch and get new reviews.

Anyway, I need the fix to Windows nsLookAndFeel before I can check in the real 
patch. Who will review 
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=32784
? How about waterson, rods?

    
    

    
  
That patch is the same as the last one except it adds back in the code that 
Hyatt took out.

I need waterson and attinasi to say whether or not their previous reviews are 
still good.

Thanks!

    
    

    
  
hyatt: IIRC you removed the call to set-background-color because you thought
your paint suppression would cover all cases? If you're okay with these changes
(hyatt), then my r= is still good.

    
    

    
  
I don't really see why this is necessary, but I have no problem with putting it 
back in.


    
    

    
  
I occasionally see some cases of transient background in my surfing (e.g, in 
some frames).

    
    

    
  
do we need this for 0.9.1 or can we move to 0.9.2?

    
    

    
  
Let's move it to 0.9.2. I'll get it in then, I promise!!!
Target Milestone: mozilla0.9.1 → mozilla0.9.2

    
    

    
  
roc, did you need to do more work on this patch?  Are you just not comfortable
with it?  I'm wondering why you were so eager to push this out.

    
    

    
  
Given Hyatt's previous checkin, this bug is minor now IMHO and doesn't need to 
be in 0.9.1.
Whiteboard: critical for mozilla 0.9.1

    
    

    
  
hyatt's checkin apparently doesn't help pages with frames so if you have a page
with frames you get the random background colors and it's really painful to see.

    
    

    
  
OK Chris, if you want it for 0.9.1, approve it on behalf of drivers, and then if
Marc is OK with it, I'll put it in.

    
    

    
  
I'm happy with it!

    
    

    
  
Alright, up to you Mr Blizzard :-).

    
    

    
  
a=blizzard for 0.9.1

    
    

    
  
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED

    
    

    
  
I am still seeing the WARNINGs when running: Debug -> Viewer Demos -> #13 DHTML

    
    

    
  
The warning I am referring to here is the following one (it has to do with the 
fact that an unnecessary request to set a transparent background is made). It is 
pretty much harmless but could suggest that unnecessary calls are done 
somewhere. However, since frames set the backgroung without testing, it is 
probably good to have the test as it is done now (with this warning) rather than 
at each calling site. 

void
nsFrame::SetDefaultBackgroundColor(nsIPresContext* aPresContext)
{
  nsStyleColor color;
  mStyleContext->GetStyle(eStyleStruct_Color, color);

  if ((color.mBackgroundFlags & NS_STYLE_BG_COLOR_TRANSPARENT) != 0) {
    NS_WARNING("Frame setting default background color but it has transparent 
background!");
  } else {
    nsCOMPtr<nsIPresShell> shell;
    aPresContext->GetShell(getter_AddRefs(shell));

    if (shell) {
      nsCOMPtr<nsIViewManager> vm;
      shell->GetViewManager(getter_AddRefs(vm));

      if (vm) {
        vm->SetDefaultBackgroundColor(color.mBackgroundColor);
      }
    }
  }
}

    
    

    
  
I think it's OK to mark it verified - no longer flashing colors.
Tested on Linux 2001062021.

Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: