From c19d7b99a381779dda0f5c07b15b1034f0e48f02 Mon Sep 17 00:00:00 2001 From: "dtownsend%oxymoronical.com" Date: Thu, 23 Aug 2007 09:00:00 +0000 Subject: [PATCH] Bug 317433: Richlistbox attempts to give focus to hidden items. p=Simon Bunzli r=enndeakin --- toolkit/content/tests/widgets/Makefile.in | 2 + .../tests/widgets/test_hiddenitems.xul | 89 ++++++++++++ .../tests/widgets/test_hiddenpaging.xul | 128 ++++++++++++++++++ toolkit/content/widgets/listbox.xml | 78 +++++++++-- toolkit/content/widgets/richlistbox.xml | 21 +-- 5 files changed, 293 insertions(+), 25 deletions(-) create mode 100644 toolkit/content/tests/widgets/test_hiddenitems.xul create mode 100644 toolkit/content/tests/widgets/test_hiddenpaging.xul diff --git a/toolkit/content/tests/widgets/Makefile.in b/toolkit/content/tests/widgets/Makefile.in index c9ef667e254..e4833270881 100644 --- a/toolkit/content/tests/widgets/Makefile.in +++ b/toolkit/content/tests/widgets/Makefile.in @@ -68,6 +68,8 @@ _TEST_FILES = test_bug360220.xul \ test_datepicker.xul \ test_timepicker.xul \ xul_selectcontrol.js \ + test_hiddenitems.xul \ + test_hiddenpaging.xul \ $(NULL) ifeq (,$(filter mac cocoa,$(MOZ_WIDGET_TOOLKIT))) diff --git a/toolkit/content/tests/widgets/test_hiddenitems.xul b/toolkit/content/tests/widgets/test_hiddenitems.xul new file mode 100644 index 00000000000..126b1b2ecb9 --- /dev/null +++ b/toolkit/content/tests/widgets/test_hiddenitems.xul @@ -0,0 +1,89 @@ + + + + + + + diff --git a/toolkit/content/tests/widgets/test_hiddenpaging.xul b/toolkit/content/tests/widgets/test_hiddenpaging.xul new file mode 100644 index 00000000000..3c106a8c664 --- /dev/null +++ b/toolkit/content/tests/widgets/test_hiddenpaging.xul @@ -0,0 +1,128 @@ + + + + + + + diff --git a/toolkit/content/widgets/listbox.xml b/toolkit/content/widgets/listbox.xml index 199252f2635..36b26aaf400 100644 --- a/toolkit/content/widgets/listbox.xml +++ b/toolkit/content/widgets/listbox.xml @@ -99,8 +99,9 @@ insertItemAt(aIndex, aLabel, aValue) /** Scroll up/down one page - * @param aDirection - specifies scrolling direction, should be either -1 - or 1 */ + * @param aDirection - specifies scrolling direction, should be either -1 or 1 + * @return the number of elements the selection scrolled + */ scrollOnePage(aDirection) /** Fire "select" event */ @@ -334,12 +335,15 @@ // Don't use clearSelection() because it causes a lot of noise // with respect to selection removed notifications used by the // accessibility API support. + var userSelecting = this._userSelecting; + this._userSelecting = false; // that's US automatically unselecting for (; currentItem; currentItem = this.getNextItem(currentItem, 1)) this.removeItemFromSelection(currentItem); for (currentItem = this.getItemAtIndex(0); currentItem != aStartItem; currentItem = this.getNextItem(currentItem, 1)) this.removeItemFromSelection(currentItem); + this._userSelecting = userSelecting; this._suppressOnSelect = suppressSelect; @@ -478,8 +482,13 @@ newIndex = numItems - 1; var newItem = this.getItemAtIndex(newIndex); + // make sure that the item is actually visible/selectable + if (this._userSelecting && newItem && !this._canUserSelect(newItem)) + newItem = + aOffset > 0 ? this.getNextItem(newItem, 1) || this.getPreviousItem(newItem, 1) : + this.getPreviousItem(newItem, 1) || this.getNextItem(newItem, 1); if (newItem) { - this.ensureIndexIsVisible(newIndex); + this.ensureIndexIsVisible(this.getIndexOfItem(newItem)); if (aIsSelectingRange) this.selectItemRange(null, newItem); else if (aIsSelecting) @@ -500,7 +509,8 @@ while (aStartItem) { aStartItem = aStartItem.nextSibling; if (aStartItem && aStartItem instanceof - Components.interfaces.nsIDOMXULSelectControlItemElement) { + Components.interfaces.nsIDOMXULSelectControlItemElement && + (!this._userSelecting || this._canUserSelect(aStartItem))) { --aDelta; if (aDelta == 0) return aStartItem; @@ -518,7 +528,8 @@ while (aStartItem) { aStartItem = aStartItem.previousSibling; if (aStartItem && aStartItem instanceof - Components.interfaces.nsIDOMXULSelectControlItemElement) { + Components.interfaces.nsIDOMXULSelectControlItemElement && + (!this._userSelecting || this._canUserSelect(aStartItem))) { --aDelta; if (aDelta == 0) return aStartItem; @@ -529,6 +540,28 @@ + + + + + + + + + + + + + + + @@ -538,6 +571,7 @@ false + false null null null @@ -545,22 +579,22 @@ = maxTop && maxTop > this.currentIndex) { - newTop = maxTop; + var maxTop = this.getRowCount() - this.getNumberOfVisibleRows(); + for (i = this.getRowCount(); i >= 0 && i > maxTop; i--) { + item = this.getItemAtIndex(i); + if (item && !this._canUserSelect(item)) + maxTop--; } + if (newTop >= maxTop) + newTop = maxTop; } - else if (newTop < 0) + if (newTop < 0) newTop = 0; this.scrollToIndex(newTop); return pageOffset; @@ -924,6 +970,7 @@ var control = this.control; if (!control || control.disabled) return; + control._userSelecting = true; if (control.selType != "multiple") { control.selectItem(this); } @@ -945,6 +992,7 @@ // doesn't de- and reselect this item if it is selected control.selectItemRange(this, this); } + control._userSelecting = false; ]]> diff --git a/toolkit/content/widgets/richlistbox.xml b/toolkit/content/widgets/richlistbox.xml index 47b8be5f316..75a4f4a3f99 100644 --- a/toolkit/content/widgets/richlistbox.xml +++ b/toolkit/content/widgets/richlistbox.xml @@ -265,19 +265,20 @@ // (including the currently selected one), and determine // the index of the first one lying (partially) outside var height = this.scrollBoxObject.height; - var border = this.currentItem.boxObject.y; + var startBorder = this.currentItem.boxObject.y; if (aDirection == -1) - border += this.currentItem.boxObject.height; + startBorder += this.currentItem.boxObject.height; + var index = this.currentIndex; - while (0 <= index && index < children.length) { - var border2 = children[index].boxObject.y; - if (aDirection == -1) - border2 += children[index].boxObject.height; - if ((border2 - border) * aDirection > height) - break; - index += aDirection; + for (var ix = index; 0 <= ix && ix < children.length; ix += aDirection) { + var boxObject = children[ix].boxObject; + if (boxObject.height == 0) + continue; // hidden children have a y of 0 + var endBorder = boxObject.y + (aDirection == -1 ? boxObject.height : 0); + if ((endBorder - startBorder) * aDirection > height) + break; // we've reached the desired distance + index = ix; } - index -= aDirection; return index != this.currentIndex ? index - this.currentIndex : aDirection; ]]>