Bug 400226. Do text redisplay as soon as it's safe, not after a trip to the event loop. r+sr=mats

This commit is contained in:
Boris Zbarsky 2009-02-17 22:27:25 -05:00
parent 6cff212e17
commit b0794d1840
8 changed files with 79 additions and 71 deletions

View File

@ -241,20 +241,23 @@ nsHTMLSelectElement::InsertOptionsIntoList(nsIContent* aOptions,
// Get the frame stuff for notification. No need to flush here
// since if there's no frame for the select yet the select will
// get into the right state once it's created.
nsISelectControlFrame* selectFrame = GetSelectFrame();
nsPresContext *presContext = nsnull;
if (selectFrame) {
presContext = GetPresContext();
}
nsISelectControlFrame* selectFrame = nsnull;
nsWeakFrame weakSelectFrame;
PRBool didGetFrame = PR_FALSE;
// Actually select the options if the added options warrant it
nsCOMPtr<nsIDOMNode> optionNode;
nsCOMPtr<nsIDOMHTMLOptionElement> option;
for (PRInt32 i=aListIndex;i<insertIndex;i++) {
// Notify the frame that the option is added
if (!didGetFrame || (selectFrame && !weakSelectFrame.IsAlive())) {
selectFrame = GetSelectFrame();
weakSelectFrame = do_QueryFrame(selectFrame);
didGetFrame = PR_TRUE;
}
if (selectFrame) {
selectFrame->AddOption(presContext, i);
selectFrame->AddOption(i);
}
Item(i, getter_AddRefs(optionNode));
@ -273,8 +276,7 @@ nsHTMLSelectElement::InsertOptionsIntoList(nsIContent* aOptions,
// This is sort of a hack ... we need to notify that the option was
// set and change selectedIndex even though we didn't really change
// its value.
OnOptionSelected(selectFrame, presContext, i, PR_TRUE, PR_FALSE,
PR_FALSE);
OnOptionSelected(selectFrame, i, PR_TRUE, PR_FALSE, PR_FALSE);
}
}
}
@ -299,9 +301,9 @@ nsHTMLSelectElement::RemoveOptionsFromList(nsIContent* aOptions,
// Tell the widget we removed the options
nsISelectControlFrame* selectFrame = GetSelectFrame();
if (selectFrame) {
nsPresContext *presContext = GetPresContext();
nsAutoScriptBlocker scriptBlocker;
for (int i = aListIndex; i < aListIndex + numRemoved; ++i) {
selectFrame->RemoveOption(presContext, i);
selectFrame->RemoveOption(i);
}
}
@ -805,7 +807,6 @@ nsHTMLSelectElement::IsOptionSelectedByIndex(PRInt32 aIndex)
void
nsHTMLSelectElement::OnOptionSelected(nsISelectControlFrame* aSelectFrame,
nsPresContext* aPresContext,
PRInt32 aIndex,
PRBool aSelected,
PRBool aChangeOptionState,
@ -830,7 +831,7 @@ nsHTMLSelectElement::OnOptionSelected(nsISelectControlFrame* aSelectFrame,
// Let the frame know too
if (aSelectFrame) {
aSelectFrame->OnOptionSelected(aPresContext, aIndex, aSelected);
aSelectFrame->OnOptionSelected(aIndex, aSelected);
}
}
@ -921,9 +922,8 @@ nsHTMLSelectElement::SetOptionsSelectedByIndex(PRInt32 aStartIndex,
PRBool optionsDeselected = PR_FALSE;
nsISelectControlFrame *selectFrame = nsnull;
PRBool did_get_frame = PR_FALSE;
nsPresContext *presContext = GetPresContext();
PRBool didGetFrame = PR_FALSE;
nsWeakFrame weakSelectFrame;
if (aIsSelected) {
// Setting selectedIndex to an out-of-bounds index means -1. (HTML5)
@ -981,11 +981,10 @@ nsHTMLSelectElement::SetOptionsSelectedByIndex(PRInt32 aStartIndex,
// force it to be created just to notify it about a change
// in the select.
selectFrame = GetSelectFrame();
weakSelectFrame = do_QueryFrame(selectFrame);
didGetFrame = PR_TRUE;
did_get_frame = PR_TRUE;
OnOptionSelected(selectFrame, presContext, optIndex, PR_TRUE,
PR_TRUE, aNotify);
OnOptionSelected(selectFrame, optIndex, PR_TRUE, PR_TRUE, aNotify);
optionsSelected = PR_TRUE;
}
}
@ -1008,17 +1007,18 @@ nsHTMLSelectElement::SetOptionsSelectedByIndex(PRInt32 aStartIndex,
PRBool isSelected = PR_FALSE;
option->GetSelected(&isSelected);
if (isSelected) {
if (!did_get_frame) {
if (!didGetFrame || (selectFrame && !weakSelectFrame.IsAlive())) {
// To notify the frame if anything gets changed, don't
// flush, if the frame doesn't exist we don't need to
// create it just to tell it about this change.
selectFrame = GetSelectFrame();
weakSelectFrame = do_QueryFrame(selectFrame);
did_get_frame = PR_TRUE;
didGetFrame = PR_TRUE;
}
OnOptionSelected(selectFrame, presContext, optIndex, PR_FALSE,
PR_TRUE, aNotify);
OnOptionSelected(selectFrame, optIndex, PR_FALSE, PR_TRUE,
aNotify);
optionsDeselected = PR_TRUE;
// Only need to deselect one option if not multiple
@ -1050,17 +1050,17 @@ nsHTMLSelectElement::SetOptionsSelectedByIndex(PRInt32 aStartIndex,
PRBool isSelected = PR_FALSE;
option->GetSelected(&isSelected);
if (isSelected) {
if (!did_get_frame) {
if (!didGetFrame || (selectFrame && !weakSelectFrame.IsAlive())) {
// To notify the frame if anything gets changed, don't
// flush, if the frame doesn't exist we don't need to
// create it just to tell it about this change.
selectFrame = GetSelectFrame();
weakSelectFrame = do_QueryFrame(selectFrame);
did_get_frame = PR_TRUE;
didGetFrame = PR_TRUE;
}
OnOptionSelected(selectFrame, presContext, optIndex, PR_FALSE,
PR_TRUE, aNotify);
OnOptionSelected(selectFrame, optIndex, PR_FALSE, PR_TRUE, aNotify);
optionsDeselected = PR_TRUE;
}
}

View File

@ -346,7 +346,6 @@ protected:
* Called to trigger notifications of frames and fixing selected index
*
* @param aSelectFrame the frame for this content (could be null)
* @param aPresContext the current pres context
* @param aIndex the index that was selected or deselected
* @param aSelected whether the index was selected or deselected
* @param aChangeOptionState if false, don't do anything to the
@ -355,7 +354,6 @@ protected:
* @param aNotify whether to notify the style system and such
*/
void OnOptionSelected(nsISelectControlFrame* aSelectFrame,
nsPresContext* aPresContext,
PRInt32 aIndex,
PRBool aSelected,
PRBool aChangeOptionState,

View File

@ -777,6 +777,7 @@ nsComboboxControlFrame::GetDropDown()
NS_IMETHODIMP
nsComboboxControlFrame::RedisplaySelectedText()
{
nsAutoScriptBlocker scriptBlocker;
return RedisplayText(mListControlFrame->GetSelectedIndex());
}
@ -805,10 +806,14 @@ nsComboboxControlFrame::RedisplayText(PRInt32 aIndex)
// displaying the wrong text.
mRedisplayTextEvent.Revoke();
NS_ASSERTION(!nsContentUtils::IsSafeToRunScript(),
"If we happen to run our redisplay event now, we might kill "
"ourselves!");
nsRefPtr<RedisplayTextEvent> event = new RedisplayTextEvent(this);
rv = NS_DispatchToCurrentThread(event);
if (NS_SUCCEEDED(rv))
mRedisplayTextEvent = event;
mRedisplayTextEvent = event;
if (!nsContentUtils::AddScriptRunner(event))
mRedisplayTextEvent.Forget();
}
return rv;
}
@ -874,19 +879,19 @@ nsComboboxControlFrame::DoneAddingChildren(PRBool aIsDone)
}
NS_IMETHODIMP
nsComboboxControlFrame::AddOption(nsPresContext* aPresContext, PRInt32 aIndex)
nsComboboxControlFrame::AddOption(PRInt32 aIndex)
{
if (aIndex <= mDisplayedIndex) {
++mDisplayedIndex;
}
nsListControlFrame* lcf = static_cast<nsListControlFrame*>(mDropdownFrame);
return lcf->AddOption(aPresContext, aIndex);
return lcf->AddOption(aIndex);
}
NS_IMETHODIMP
nsComboboxControlFrame::RemoveOption(nsPresContext* aPresContext, PRInt32 aIndex)
nsComboboxControlFrame::RemoveOption(PRInt32 aIndex)
{
if (mListControlFrame->GetNumberOfOptions() > 0) {
if (aIndex < mDisplayedIndex) {
@ -902,7 +907,7 @@ nsComboboxControlFrame::RemoveOption(nsPresContext* aPresContext, PRInt32 aIndex
}
nsListControlFrame* lcf = static_cast<nsListControlFrame*>(mDropdownFrame);
return lcf->RemoveOption(aPresContext, aIndex);
return lcf->RemoveOption(aIndex);
}
NS_IMETHODIMP
@ -919,6 +924,7 @@ nsComboboxControlFrame::GetOptionSelected(PRInt32 aIndex, PRBool* aValue)
NS_IMETHODIMP
nsComboboxControlFrame::OnSetSelectedIndex(PRInt32 aOldIndex, PRInt32 aNewIndex)
{
nsAutoScriptBlocker scriptBlocker;
RedisplayText(aNewIndex);
NS_ASSERTION(mDropdownFrame, "No dropdown frame!");
@ -1433,21 +1439,23 @@ nsIScrollableView* nsComboboxControlFrame::GetScrollableView()
// being selected or not selected
//---------------------------------------------------------
NS_IMETHODIMP
nsComboboxControlFrame::OnOptionSelected(nsPresContext* aPresContext,
PRInt32 aIndex,
PRBool aSelected)
nsComboboxControlFrame::OnOptionSelected(PRInt32 aIndex, PRBool aSelected)
{
if (mDroppedDown) {
nsISelectControlFrame *selectFrame = do_QueryFrame(mListControlFrame);
if (selectFrame) {
selectFrame->OnOptionSelected(aPresContext, aIndex, aSelected);
selectFrame->OnOptionSelected(aIndex, aSelected);
}
} else {
if (aSelected) {
nsAutoScriptBlocker blocker;
RedisplayText(aIndex);
} else {
nsWeakFrame weakFrame(this);
RedisplaySelectedText();
FireValueChangeEvent(); // Fire after old option is unselected
if (weakFrame.IsAlive()) {
FireValueChangeEvent(); // Fire after old option is unselected
}
}
}

View File

@ -175,18 +175,19 @@ public:
virtual void RollupFromList();
virtual void AbsolutelyPositionDropDown();
virtual PRInt32 GetIndexOfDisplayArea();
/**
* @note This method might destroy |this|.
*/
NS_IMETHOD RedisplaySelectedText();
virtual PRInt32 UpdateRecentIndex(PRInt32 aIndex);
virtual void OnContentReset();
// nsISelectControlFrame
NS_IMETHOD AddOption(nsPresContext* aPresContext, PRInt32 index);
NS_IMETHOD RemoveOption(nsPresContext* aPresContext, PRInt32 index);
NS_IMETHOD AddOption(PRInt32 index);
NS_IMETHOD RemoveOption(PRInt32 index);
NS_IMETHOD GetOptionSelected(PRInt32 aIndex, PRBool* aValue);
NS_IMETHOD DoneAddingChildren(PRBool aIsDone);
NS_IMETHOD OnOptionSelected(nsPresContext* aPresContext,
PRInt32 aIndex,
PRBool aSelected);
NS_IMETHOD OnOptionSelected(PRInt32 aIndex, PRBool aSelected);
NS_IMETHOD OnSetSelectedIndex(PRInt32 aOldIndex, PRInt32 aNewIndex);
//nsIRollupListener

View File

@ -82,7 +82,9 @@ public:
virtual void RollupFromList() = 0;
/**
* Redisplay the selected text (will do nothing if text has not changed)
* Redisplay the selected text (will do nothing if text has not changed).
* This method might destroy this frame or any others that happen to be
* around. It might even run script.
*/
NS_IMETHOD RedisplaySelectedText() = 0;

View File

@ -46,7 +46,7 @@ class nsIDOMHTMLOptionElement;
/**
* nsISelectControlFrame is the interface for combo boxes and listboxes
*/
class nsISelectControlFrame
class nsISelectControlFrame : public nsQueryFrame
{
public:
NS_DECLARE_FRAME_ACCESSOR(nsISelectControlFrame)
@ -55,13 +55,13 @@ public:
* Adds an option to the list at index
*/
NS_IMETHOD AddOption(nsPresContext* aPresContext, PRInt32 index) = 0;
NS_IMETHOD AddOption(PRInt32 index) = 0;
/**
* Removes the option at index
* Removes the option at index. The caller must have a live script
* blocker while calling this method.
*/
NS_IMETHOD RemoveOption(nsPresContext* aPresContext, PRInt32 index) = 0;
NS_IMETHOD RemoveOption(PRInt32 index) = 0;
/**
* Sets the select state of the option at index
@ -77,12 +77,11 @@ public:
/**
* Notify the frame when an option is selected
*/
NS_IMETHOD OnOptionSelected(nsPresContext* aPresContext,
PRInt32 aIndex,
PRBool aSelected) = 0;
NS_IMETHOD OnOptionSelected(PRInt32 aIndex, PRBool aSelected) = 0;
/**
* Notify the frame when selectedIndex was changed
* Notify the frame when selectedIndex was changed. This might
* destroy the frame.
*/
NS_IMETHOD OnSetSelectedIndex(PRInt32 aOldIndex, PRInt32 aNewIndex) = 0;

View File

@ -1273,9 +1273,7 @@ nsListControlFrame::IsContentSelectedByIndex(PRInt32 aIndex) const
}
NS_IMETHODIMP
nsListControlFrame::OnOptionSelected(nsPresContext* aPresContext,
PRInt32 aIndex,
PRBool aSelected)
nsListControlFrame::OnOptionSelected(PRInt32 aIndex, PRBool aSelected)
{
if (aSelected) {
ScrollToIndex(aIndex);
@ -1466,7 +1464,7 @@ nsListControlFrame::DoneAddingChildren(PRBool aIsDone)
}
NS_IMETHODIMP
nsListControlFrame::AddOption(nsPresContext* aPresContext, PRInt32 aIndex)
nsListControlFrame::AddOption(PRInt32 aIndex)
{
#ifdef DO_REFLOW_DEBUG
printf("---- Id: %d nsLCF %p Added Option %d\n", mReflowId, this, aIndex);
@ -1493,7 +1491,7 @@ nsListControlFrame::AddOption(nsPresContext* aPresContext, PRInt32 aIndex)
}
NS_IMETHODIMP
nsListControlFrame::RemoveOption(nsPresContext* aPresContext, PRInt32 aIndex)
nsListControlFrame::RemoveOption(PRInt32 aIndex)
{
// Need to reset if we're a dropdown
if (IsInDropDownMode()) {
@ -1585,15 +1583,15 @@ nsListControlFrame::UpdateSelection()
{
if (mIsAllFramesHere) {
// if it's a combobox, display the new text
nsWeakFrame weakFrame(this);
if (mComboboxFrame) {
mComboboxFrame->RedisplaySelectedText();
}
// if it's a listbox, fire on change
else if (mIsAllContentHere) {
nsWeakFrame weakFrame(this);
FireOnChange();
return weakFrame.IsAlive();
}
return weakFrame.IsAlive();
}
return PR_TRUE;
}
@ -1608,11 +1606,15 @@ nsListControlFrame::ComboboxFinish(PRInt32 aIndex)
PRInt32 displayIndex = mComboboxFrame->GetIndexOfDisplayArea();
nsWeakFrame weakFrame(this);
if (displayIndex != aIndex) {
mComboboxFrame->RedisplaySelectedText();
mComboboxFrame->RedisplaySelectedText(); // might destroy us
}
mComboboxFrame->RollupFromList(); // might destroy us
if (weakFrame.IsAlive() && mComboboxFrame) {
mComboboxFrame->RollupFromList(); // might destroy us
}
}
}

View File

@ -186,8 +186,8 @@ public:
virtual void OnContentReset();
// nsISelectControlFrame
NS_IMETHOD AddOption(nsPresContext* aPresContext, PRInt32 index);
NS_IMETHOD RemoveOption(nsPresContext* aPresContext, PRInt32 index);
NS_IMETHOD AddOption(PRInt32 index);
NS_IMETHOD RemoveOption(PRInt32 index);
NS_IMETHOD GetOptionSelected(PRInt32 aIndex, PRBool* aValue);
NS_IMETHOD DoneAddingChildren(PRBool aIsDone);
@ -195,9 +195,7 @@ public:
* Gets the content (an option) by index and then set it as
* being selected or not selected.
*/
NS_IMETHOD OnOptionSelected(nsPresContext* aPresContext,
PRInt32 aIndex,
PRBool aSelected);
NS_IMETHOD OnOptionSelected(PRInt32 aIndex, PRBool aSelected);
NS_IMETHOD OnSetSelectedIndex(PRInt32 aOldIndex, PRInt32 aNewIndex);
// mouse event listeners (both might destroy |this|)