Closed Bug 78412 Opened 19 years ago Closed 19 years ago
Moving browser window corner off screen causes window to be erased
Moving browser window corner off screen causes window to be erased. Steps to reproduce: 1. Run mfcEmbed.exe 2. Move window so that is clipped on at least two sides. (drag title bar towards any bottom corner) 3. move window back: observe: window is erased in the non-clipped region. repaint works fine if you clip just one side of the window at a time.
Due to interest from the very important embedding customer I'm setting --- or Future target milestone to 0.9.1
Target Milestone: --- → mozilla0.9.1
Hmm, code inspection makes the culprit likely to be this: http://lxr.mozilla.org/seamonkey/source/embedding/browser/webBrowser/nsWebBrowse r.cpp#1334 The reason for the problem is webbrowser paints a rectangle in response to an NS_PAINT event. The rectangle is the bounding rectangle of the invalidated region. When the left or bottom edge is invalidated, there is no problem because the region and the bounding rectangle are the same so we paint correctly. However, when the bottom and left hand edges are invalidated together, the invalid region is an 'L' shape and the bounding rectangle is the whole widget. This means we end up blatting out everthing on the page, not just the invalid area. We should change the code to just paint the region.
On Win32 it looks as though the nsPaintEvent::region variable passed to our handler just contains garbage so I was unable to use that to obtain the region from that. Fortunately, the paint event also passes a rendering context and that has the clipping region correctly set on it. I've updated the code to use paint to that rendering context rather than the one stored in the web browser.
This won't work on Linux since the event.region is null. I suspect that the mac might have similar problems. I'm not sure why this is a bug since it will only fill in the area of the window before we've loaded anything. There probably can't be too much flashing there, right? What is the visual effect here?
The effect is that it blanks out the entire browser when you move the window back on screen and doesn't repaint it until the next update whenever that is. My patch doesn't use the region attribute because it is null or garbage on Win32 too. Does it work on any platform? Instead the patch uses the renderingContext attribute, calling FillRect on that because its clipping region ensures only the invalidated area will be blanked out. This would work on all platforms right?
This doesn't work on linux. I tried it.
What doesn't work on Linux, the bug or the patch? If the patch are you seeing strange behaviour that wasn't there before?
The patch doesn't work. It doesn't redraw the area that's exposed - ever.
FWIW, the patch does work on Mac. Not that that means anything until it works everywhere, but...
Back to the drawing board... I've done some further investigation as to why the renderingContext sent with the nsPaintEvent doesn't work in GTK. It appears that it has something to do with the way the widget is initially created. During creation, nsWebBrowser creates a device context and initialises the internal widget with it. This seems to mess up things later on, perhaps because the rendering context returned in the paint event is not associated with the device context set on the widget during creation. I also suspect this DC confusion is the cause of a mysterious black/gray rectangle in the top left of the browser window that momentarily appears during resizing. I looked at the blame log and the device context creation/setting code seems to be all Travis-era so I've removed it all. I've also removed the mRC & mDC member variables. This appears to make GTK happy. A patch follows. I have yet to test other platforms with it.
Works on Win32 as well...
Works on Mac as well. Can I have reviews and comments please?
I'm not the best reviewer for the logic, but the code looks fine. r=valeski
So, that device context that was being made was not only uneeded, but harmful. Good riddance! r=ccarlen
The old patch didn't patch cleanly anymore. The updated patch includes a comment change in the header file that I would have requested in an sr= anyway so you might want to use it instead. Thanks for the hard work adam, it works great! Use that patch and you can have an sr=blizzard.
Fix checked in
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
reassign qa contact to depstein
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.