83493 - [FIX]Mfcembed leaves garbage when combo is rolled up

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect, P1)

Description

[rods (gone)]

When the combo gets rolled up from the special "hook" functions (like clicking 
on the URL in MFCEmbed) sometimes it leaves garbage.

Comment 1

[rods (gone)]

Attachment: patch

Comment 2

[rods (gone)]

The problem is you can't do the roll up in the middle of a "hook" function call. 
So a timer is native created and the DealWithPopups is called there.

For example, when you click on the MFCEmbed URLbar we do the rollup but the 
SetFocus of the CBX wants to do the roll up and this is where we run into 
problems. 

Also, the CBX now needs to make sure that all the views in the content area are 
updated, not just its view. This is because there may be a absolutely 
positioned div just under the CBX. So I will attach a new patch for this.

Comment 3

[rods (gone)]

Attachment: This patch updates all the views

Comment 4

[rods (gone)]

Attachment: testcase with absolutely positioned div

Comment 5

[Kevin McCluskey (gone)]

// Make sure the content area gets updated for where the dropdown was
+ // This is only needed for embedding, the focus may go to
+ // the chrome that is not part of the Gecko system (Bug 83493)
+ nsIFrame* parent;
+ GetParentWithView(mPresContext, &parent);
+ if (parent != nsnull) {
+ nsIView* view = nsnull;
+ parent->GetView(mPresContext, &view);
+ if (view != nsnull) {
+ nsCOMPtr<nsIPresShell> presShell;
+ mPresContext->GetShell(getter_AddRefs(presShell));
+ if (presShell) {
+ nsCOMPtr<nsIViewManager> vm;
+ presShell->GetViewManager(getter_AddRefs(vm));
+ if (vm) {
+ vm->UpdateAllViews(NS_VMREFRESH_NO_SYNC);
+ }
+ }
+ }
+ }
}

I'm confused by this part of the patch. You can get the view manager by asking
the view for its viewmanager directly, there is no need to use the mPresContext,
or you can just ask the mPresContext for the viewmanager directly and eliminate
the code that gets the view from the frame.

nsIFrame* parent;
GetParentWithView(mPresContext, &parent);
if (parent != nsnull) {
nsIView* view = nsnull;
parent->GetView(mPresContext, &view);
if (view != nsnull) {
  nsCOMPtr<nsIViewManager> vm;
  if (vm) {
    vm->UpdateAllViews(NS_VMREFRESH_NO_SYNC);
  }

or 

nsCOMPtr<nsIPresShell> presShell;
mPresContext->GetShell(getter_AddRefs(presShell));
if (presShell) {
 nsCOMPtr<nsIViewManager> vm;
 presShell->GetViewManager(getter_AddRefs(vm));
 if (vm) {
  vm->UpdateAllViews(NS_VMREFRESH_NO_SYNC);
 }

Both of these will cause the entire content area to be refreshed however.
We may want to consider calculating the exact rect occupied by the drop-down if
this proves to be a performance issue.



Nit: You may want to factor the following to lines into a separate function
ScheduleRollup passing in the MsgId instead of replicating this code in several
places.
 
        gRollupMsgId = wParam;
+       ::SetTimer( ms->hwnd, NS_HOOK_TIMER_ID, 0, (TIMERPROC)HookTimerForPopups );
 

Comment 6

[Kevin McCluskey (gone)]

Oops... I missed the line to get the viewmanager

nsIFrame* parent;
GetParentWithView(mPresContext, &parent);
if (parent != nsnull) {
nsIView* view = nsnull;
parent->GetView(mPresContext, &view);
if (view != nsnull) {
+ view->GetViewManager(vm);
  nsCOMPtr<nsIViewManager> vm;
  if (vm) {
    vm->UpdateAllViews(NS_VMREFRESH_NO_SYNC);
  }

Assumming you resolve this issue, the rest of the patch looks good. 
r=[e-mail]

Comment 7

[rods (gone)]

I have made the changes for your suggestions. I will open a bug for optimizing 
the updating of all the views, that is a little bit too much to do here and now.

Comment 8

[Marc Attinasi]

sr=attinasi (based on conversation with Rod over phone)

Comment 9

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to the trunk.
(on behalf of drivers)

Comment 10

[rods (gone)]

fixed

Comment 11

[Vladimir Ermakov]

verifying build 2001-07-10-05-0.9.2 windows 2000