Closed
Bug 629823
Opened 14 years ago
Closed 14 years ago
Use the right VM for the parent view in nsViewManager::ResizeView. Assert that the root view size is in sync with the pres context visible size.
Categories
(Core :: Web Painting, defect)
Core
Web Painting
Tracking
()
RESOLVED
FIXED
mozilla2.0b12
People
(Reporter: MatsPalmgren_bugz, Assigned: MatsPalmgren_bugz)
References
(Depends on 1 open bug)
Details
Attachments
(2 files, 2 obsolete files)
4.85 KB,
patch
|
tnikkel
:
review+
|
Details | Diff | Splinter Review |
4.16 KB,
patch
|
roc
:
approval2.0+
|
Details | Diff | Splinter Review |
Follow-up from bug 617076 (the part 1 patch):
Timothy pointed out that nsViewManager::ResizeView shouldn't be called with
a root view. I added an assertion for that which revealed that it triggers
all the time so I added "NS_FRAME_NO_MOVE_VIEW | NS_FRAME_NO_SIZE_VIEW"
flags in PresShell::DoReflow to avoid that. (see patch)
Timothy said this is wrong and we should instead fix whatever makes
that happen.
Comment 2•14 years ago
|
||
Ok so nsViewManager::ResizeView is called on the root view all the time, but do we ever change the size of the root view this way? That is the case we don't want to happen.
Assignee | ||
Comment 3•14 years ago
|
||
You mean like so?
It occurs when opening Edit->Preferences on Linux:
###!!! ASSERTION: ResizeView is resizing the root view: 'view != mRootView', file view/src/nsViewManager.cpp, line 1349
#0 NS_DebugBreak_P
#1 nsViewManager::ResizeView at view/src/nsViewManager.cpp:1349
#2 PresShell::DoReflow at layout/base/nsPresShell.cpp:7869
#3 PresShell::ResizeReflowIgnoreOverride at layout/base/nsPresShell.cpp:2886
#4 DocumentViewerImpl::SizeToContent at layout/base/nsDocumentViewer.cpp:3207
#5 nsGlobalWindow::SizeToContent at dom/base/nsGlobalWindow.cpp:5263
#6 nsGlobalWindow::SizeToContent at dom/base/nsGlobalWindow.cpp:5241
#7 NS_InvokeByIndex_P
...
Comment 4•14 years ago
|
||
Yeah, I was thinking something like that. So you're saying that assertion still fires?
Comment 6•14 years ago
|
||
Ok, I see. The main reason I wanted that assertion was to make sure that the visible area stored on the prescontext is always in sync with the root view size. So I think we should just make ResizeView handle resizing a root view properly. Mats, I think you had a patch for that in the old bug?
Comment 7•14 years ago
|
||
And adding an assert that the root view's size matches the prescontext's visible area would be good, is the end PresShell::FlushPendingNotifications a good place for that?
Assignee | ||
Comment 8•14 years ago
|
||
It seems we can only assert that when (aType >= Flush_Layout) though.
Assignee: nobody → matspal
Attachment #508039 -
Attachment is obsolete: true
Attachment #508178 -
Attachment is obsolete: true
Attachment #510800 -
Flags: review?(tnikkel)
Comment 9•14 years ago
|
||
(In reply to comment #8)
> It seems we can only assert that when (aType >= Flush_Layout) though.
That makes sense because we only do the FlushDelayedResize(PR_TRUE) call if aType >= Flush_Layout (some of the time).
Comment 10•14 years ago
|
||
Comment on attachment 510800 [details] [diff] [review]
fix v2
>@@ -4906,16 +4909,27 @@ PresShell::FlushPendingNotifications(moz
>+ static_cast<nsView*>(rootView)->GetDimensions(viewSize);
Let's keep nsView inside of /view I think. So use whatever methods available on nsIView to get what you need.
Thanks Mats.
Attachment #510800 -
Flags: review?(tnikkel) → review+
Comment 11•14 years ago
|
||
Comment on attachment 510800 [details] [diff] [review]
fix v2
I actually think we should take this. If zooming is involved then we could see the same type of problem as bug 617076. And it should be safe, we execute the same code, but in the right view manager with the right app units per dev pixel value.
Attachment #510800 -
Flags: approval2.0?
Assignee | ||
Comment 12•14 years ago
|
||
with the nit fixed
Assignee | ||
Updated•14 years ago
|
Attachment #510800 -
Flags: approval2.0?
Assignee | ||
Updated•14 years ago
|
Attachment #511350 -
Flags: approval2.0?
Attachment #511350 -
Flags: approval2.0? → approval2.0+
Assignee | ||
Comment 13•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/0a9069e1e1b8
http://hg.mozilla.org/mozilla-central/rev/502b2bea2c4d
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Summary: Make nsViewManager::ResizeView assert it's not called with a root view → Use the right VM for the parent view in nsViewManager::ResizeView. Assert that the root view size is in sync with the pres context visible size.
Target Milestone: --- → mozilla2.0b12
Updated•7 years ago
|
Component: Layout: View Rendering → Layout: Web Painting
You need to log in
before you can comment on or make changes to this bug.
Description
•