Closed Bug 765643 Opened 8 years ago Closed 8 years ago

profile path in profile manager asserts error when hovering over blank

Categories

(SeaMonkey :: Startup & Profiles, defect)

x86
Windows Vista
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ewong, Assigned: ewong)

References

Details

Attachments

(1 file, 3 obsolete files)

Quoting from 670561#c20
> Sadly this has a bug; if you hover over a blank area of the tree you get a 
> blank tooltip, and additionally an assertion in debug builds.


ASSERTION: band index:'aIndex >= 0 && aIndex < PRInt32(mRows.Length())', file
           /c:/mozstuff/smbd/mozilla/layout/xul/base/src/tree/src/nsTreeContentView.cpp, line 729

then it crashes to desktop.
Attachment #633945 - Flags: review? → review?(iann_bugzilla)
are you sure you need either of aEvent.stopPropagation() or aEvent.preventDefault();?
Try something like this:

  if (row.value == -1 || row.value > tree.view.rowCount-1 || aEvent.originalTarget.localName != "treechildren")
    //aEvent.stopPropagation();
    //aEvent.preventDefault();
    return;
  }
  treeTip.label = tree.view.getItemAtIndex(row.value).tooltip;
(In reply to Philip Chee from comment #2)
> are you sure you need either of aEvent.stopPropagation() or
> aEvent.preventDefault();?
> Try something like this:
> 
>   if (row.value == -1 || row.value > tree.view.rowCount-1 ||
> aEvent.originalTarget.localName != "treechildren")
>     //aEvent.stopPropagation();
>     //aEvent.preventDefault();
>     return;
>   }
>   treeTip.label = tree.view.getItemAtIndex(row.value).tooltip;

That doesn't work.  It doesn't display any of the profiles.

if I tried:

  if (row.value >= 0) {
    treeTip.label = tree.view.getItemAtIndex(row.value).tooltip;
  }
  else {
    return;
  }

it displays the last hovered profile name.  

The stopPropagation() and the preventDefault() both stops it from displaying
an empty tooltip.
(In reply to Philip Chee from comment #2)
> are you sure you need either of aEvent.stopPropagation() or
> aEvent.preventDefault();?
You need aEvent.preventDefault(); to stop the popup from opening. (The other alternative is to return false from the event handler, but you would have to change the XUL to propagate the return value.)

Nit: patch contains too many {}s.
Blocks: 670561
Comment on attachment 633945 [details] [diff] [review]
Ensure that hovering over empty areas do not assert. (v1)

> function HandleToolTipEvent(aEvent)
> {
>   var treeTip = document.getElementById("treetip");
>   var tree = document.getElementById("profiles");
>   var row = {};
> 
>   tree.treeBoxObject.getCellAt(aEvent.clientX, aEvent.clientY, row, {}, {});
>+  if (row.value >= 0) {
>+    treeTip.label = tree.view.getItemAtIndex(row.value).tooltip;
>+  }
>+  else {
>+    aEvent.stopPropagation();
>+    aEvent.preventDefault();
>+  }
> }
I guess you could check if row.value is < 0 do your aEvent stuff and then return early. You could move your defining of treeTip and setting of its label to the end of the function. Not sure how else you can reduce the number of {}s otherwise.
r=me either way.
Attachment #633945 - Flags: review?(iann_bugzilla) → review+
Attachment #633945 - Attachment is obsolete: true
Attachment #638950 - Flags: review+
Attachment #638950 - Attachment is obsolete: true
Attachment #638986 - Flags: review+
Comment on attachment 638986 [details] [diff] [review]
Ensure that hovering over empty areas do not assert. (v3)

think I better get a second review.
Attachment #638986 - Flags: review+ → review?(iann_bugzilla)
Comment on attachment 638986 [details] [diff] [review]
Ensure that hovering over empty areas do not assert. (v3)

>+  if (row.value < 0) {
You've not removed the {, testing should have shown that.
>+    aEvent.preventDefault();
>+  else
>+    treeTip.label = tree.view.getItemAtIndex(row.value).tooltip;
> }
r=me with that fixed.
Attachment #638986 - Flags: review?(iann_bugzilla) → review+
Attachment #638986 - Attachment is obsolete: true
Attachment #639216 - Flags: review+
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/5c2016c497e7
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.