Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-28 15:23:51 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Make <select> handle changes to defaultSelected on node for which selected has not been set. Bug 395107, r+sr=sicking.

Browse Source
This commit is contained in:
bzbarsky@mit.edu 2007-12-04 08:50:32 -08:00
parent 8154ac7363
commit c446f4e760
22 changed files with 334 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
content/base/src/nsGenericElement.cpp
View File

@ -3848,6 +3848,9 @@ nsGenericElement::UnsetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
return NS_OK;
}
nsresult rv = BeforeSetAttr(aNameSpaceID, aName, nsnull, aNotify);
NS_ENSURE_SUCCESS(rv, rv);
nsIDocument *document = GetCurrentDoc();
mozAutoDocUpdate updateBatch(document, UPDATE_CONTENT_MODEL, aNotify);
if (document) {
@ -3890,7 +3893,7 @@ nsGenericElement::UnsetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
}
nsAttrValue oldValue;
nsresult rv = mAttrsAndChildren.RemoveAttrAt(index, oldValue);
rv = mAttrsAndChildren.RemoveAttrAt(index, oldValue);
NS_ENSURE_SUCCESS(rv, rv);
if (document) {
@ -3929,7 +3932,7 @@ nsGenericElement::UnsetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
nsEventDispatcher::Dispatch(this, nsnull, &mutation);
}
return NS_OK;
return AfterSetAttr(aNameSpaceID, aName, nsnull, aNotify);
}
const nsAttrName*

4
content/base/src/nsGenericElement.h
View File

@ -824,8 +824,6 @@ protected:
* @param aName the localname of the attribute being set
* @param aValue the value it's being set to. If null, the attr is being
* removed.
* // XXXbz we don't actually call this method when we're removing attrs yet.
* But we will eventually.
* @param aNotify Whether we plan to notify document observers.
*/
// Note that this is inlined so that when subclasses call it it gets
@ -845,8 +843,6 @@ protected:
* @param aName the localname of the attribute being set
* @param aValue the value it's being set to. If null, the attr is being
* removed.
* // XXXbz we don't actually call this method when we're removing attrs yet.
* But we will eventually.
* @param aNotify Whether we plan to notify document observers.
*/
// Note that this is inlined so that when subclasses call it it gets

13
content/html/content/src/nsGenericHTMLElement.cpp
View File

@ -2822,19 +2822,6 @@ nsGenericHTMLFormElement::AfterSetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
aValue, aNotify);
}
nsresult
nsGenericHTMLFormElement::UnsetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
PRBool aNotify)
{
BeforeSetAttr(aNameSpaceID, aName, nsnull, aNotify);
nsresult rv = nsGenericHTMLElement::UnsetAttr(aNameSpaceID, aName, aNotify);
AfterSetAttr(aNameSpaceID, aName, nsnull, aNotify);
return rv;
}
PRBool
nsGenericHTMLFormElement::CanBeDisabled() const
{

2
content/html/content/src/nsGenericHTMLElement.h
View File

@ -853,8 +853,6 @@ public:
PRBool aCompileEventHandlers);
virtual void UnbindFromTree(PRBool aDeep = PR_TRUE,
PRBool aNullParent = PR_TRUE);
virtual nsresult UnsetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
PRBool aNotify);
virtual PRUint32 GetDesiredIMEState();
virtual PRInt32 IntrinsicState() const;

61
content/html/content/src/nsHTMLOptionElement.cpp
View File

@ -108,6 +108,9 @@ public:
virtual nsChangeHint GetAttributeChangeHint(const nsIAtom* aAttribute,
PRInt32 aModType) const;
virtual nsresult BeforeSetAttr(PRInt32 aNamespaceID, nsIAtom* aName,
const nsAString* aValue, PRBool aNotify);
// nsIOptionElement
NS_IMETHOD SetSelectedInternal(PRBool aValue, PRBool aNotify);
@ -127,6 +130,10 @@ protected:
PRPackedBool mSelectedChanged;
PRPackedBool mIsSelected;
// True only while we're under the SetOptionsSelectedByIndex call when our
// "selected" attribute is changing and mSelectedChanged is false.
PRPackedBool mIsInSetDefaultSelected;
};
nsGenericHTMLElement*
@ -156,7 +163,8 @@ NS_NewHTMLOptionElement(nsINodeInfo *aNodeInfo, PRBool aFromParser)
nsHTMLOptionElement::nsHTMLOptionElement(nsINodeInfo *aNodeInfo)
: nsGenericHTMLElement(aNodeInfo),
mSelectedChanged(PR_FALSE),
mIsSelected(PR_FALSE)
mIsSelected(PR_FALSE),
mIsInSetDefaultSelected(PR_FALSE)
{
}
@ -206,7 +214,9 @@ nsHTMLOptionElement::SetSelectedInternal(PRBool aValue, PRBool aNotify)
mSelectedChanged = PR_TRUE;
mIsSelected = aValue;
if (aNotify) {
// When mIsInSetDefaultSelected is true, the notification will be handled by
// SetAttr/UnsetAttr.
if (aNotify && !mIsInSetDefaultSelected) {
nsIDocument* document = GetCurrentDoc();
if (document) {
mozAutoDocUpdate upd(document, UPDATE_CONTENT_STATE, aNotify);
@ -328,6 +338,53 @@ nsHTMLOptionElement::GetAttributeChangeHint(const nsIAtom* aAttribute,
return retval;
}
nsresult
nsHTMLOptionElement::BeforeSetAttr(PRInt32 aNamespaceID, nsIAtom* aName,
const nsAString* aValue, PRBool aNotify)
{
nsresult rv = nsGenericHTMLElement::BeforeSetAttr(aNamespaceID, aName,
aValue, aNotify);
NS_ENSURE_SUCCESS(rv, rv);
if (aNamespaceID != kNameSpaceID_None || aName != nsGkAtoms::selected ||
mSelectedChanged) {
return NS_OK;
}
// We just changed out selected state (since we look at the "selected"
// attribute when mSelectedChanged is false. Let's tell our select about
// it.
nsCOMPtr<nsISelectElement> selectInt = do_QueryInterface(GetSelect());
if (!selectInt) {
return NS_OK;
}
// Note that at this point mSelectedChanged is false and as long as that's
// true it doesn't matter what value mIsSelected has.
NS_ASSERTION(!mSelectedChanged, "Shouldn't be here");
PRBool newSelected = (aValue != nsnull);
PRBool inSetDefaultSelected = mIsInSetDefaultSelected;
mIsInSetDefaultSelected = PR_TRUE;
PRInt32 index;
GetIndex(&index);
// This should end up calling SetSelectedInternal, which we will allow to
// take effect so that parts of SetOptionsSelectedByIndex that might depend
// on it working don't get confused.
rv = selectInt->SetOptionsSelectedByIndex(index, index, newSelected,
PR_FALSE, PR_TRUE, aNotify,
nsnull);
// Now reset our members; when we finish the attr set we'll end up with the
// rigt selected state.
mIsInSetDefaultSelected = inSetDefaultSelected;
mSelectedChanged = PR_FALSE;
// mIsSelected doesn't matter while mSelectedChanged is false
return rv;
}
NS_IMETHODIMP
nsHTMLOptionElement::GetText(nsAString& aText)
{

32
content/html/content/src/nsHTMLSelectElement.cpp
View File

@ -271,7 +271,8 @@ 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);
OnOptionSelected(selectFrame, presContext, i, PR_TRUE, PR_FALSE,
PR_FALSE);
}
}
}
@ -829,6 +830,7 @@ nsHTMLSelectElement::OnOptionSelected(nsISelectControlFrame* aSelectFrame,
nsPresContext* aPresContext,
PRInt32 aIndex,
PRBool aSelected,
PRBool aChangeOptionState,
PRBool aNotify)
{
// Set the selected index
@ -838,12 +840,14 @@ nsHTMLSelectElement::OnOptionSelected(nsISelectControlFrame* aSelectFrame,
FindSelectedIndex(aIndex+1);
}
// Tell the option to get its bad self selected
nsCOMPtr<nsIDOMNode> option;
Item(aIndex, getter_AddRefs(option));
if (option) {
nsCOMPtr<nsIOptionElement> optionElement(do_QueryInterface(option));
optionElement->SetSelectedInternal(aSelected, aNotify);
if (aChangeOptionState) {
// Tell the option to get its bad self selected
nsCOMPtr<nsIDOMNode> option;
Item(aIndex, getter_AddRefs(option));
if (option) {
nsCOMPtr<nsIOptionElement> optionElement(do_QueryInterface(option));
optionElement->SetSelectedInternal(aSelected, aNotify);
}
}
// Let the frame know too
@ -885,6 +889,11 @@ nsHTMLSelectElement::FindSelectedIndex(PRInt32 aStartIndex)
// changed regardless of whether it is selected or not.
// Generally the UI passes TRUE and JS passes FALSE.
// (setDisabled currently is the opposite)
//
// XXXbz the above comment is pretty confusing. Maybe we should actually
// document the args to this function too, in addition to documenting what
// things might end up looking like? In particular, pay attention to the
// setDisabled vs checkDisabled business.
NS_IMETHODIMP
nsHTMLSelectElement::SetOptionsSelectedByIndex(PRInt32 aStartIndex,
PRInt32 aEndIndex,
@ -996,7 +1005,8 @@ nsHTMLSelectElement::SetOptionsSelectedByIndex(PRInt32 aStartIndex,
did_get_frame = PR_TRUE;
OnOptionSelected(selectFrame, presContext, optIndex, PR_TRUE, aNotify);
OnOptionSelected(selectFrame, presContext, optIndex, PR_TRUE,
PR_TRUE, aNotify);
optionsSelected = PR_TRUE;
}
}
@ -1028,7 +1038,8 @@ nsHTMLSelectElement::SetOptionsSelectedByIndex(PRInt32 aStartIndex,
did_get_frame = PR_TRUE;
}
OnOptionSelected(selectFrame, presContext, optIndex, PR_FALSE, aNotify);
OnOptionSelected(selectFrame, presContext, optIndex, PR_FALSE,
PR_TRUE, aNotify);
optionsDeselected = PR_TRUE;
// Only need to deselect one option if not multiple
@ -1069,7 +1080,8 @@ nsHTMLSelectElement::SetOptionsSelectedByIndex(PRInt32 aStartIndex,
did_get_frame = PR_TRUE;
}
OnOptionSelected(selectFrame, presContext, optIndex, PR_FALSE, aNotify);
OnOptionSelected(selectFrame, presContext, optIndex, PR_FALSE,
PR_TRUE, aNotify);
optionsDeselected = PR_TRUE;
}
}

1
content/html/content/test/Makefile.in
View File

@ -103,6 +103,7 @@ _TEST_FILES = test_bug589.html \
bug392567.jar \
bug392567.jar^headers^ \
test_bug394700.html \
test_bug395107.html \
test_bug401160.xhtml \
$(NULL)

109
content/html/content/test/test_bug395107.html Normal file
View File

@ -0,0 +1,109 @@
<!DOCTYPE HTML>
<html>
<!--
https://bugzilla.mozilla.org/show_bug.cgi?id=395107
-->
<head>
<title>Test for Bug 395107</title>
<script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
<script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
</head>
<body>
<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=395107">Mozilla Bug 395107</a>
<p id="display"></p>
<div id="content" style="display: none">
</div>
<pre id="test">
<script class="testbody" type="text/javascript">
/** Test for Bug 395107 **/
var testNumber = 0;
function assertSelected(aOption, aExpectDefaultSelected, aExpectSelected) {
++testNumber;
is(aOption.defaultSelected, aExpectDefaultSelected,
"Asserting default-selected state for option " + testNumber);
is(aOption.selected, aExpectSelected,
"Asserting selected state for option " + testNumber);
}
function assertSame(aSel1, aSel2Str, aTestNumber) {
var div = document.createElement("div");
div.innerHTML = aSel2Str;
sel2 = div.firstChild;
is(aSel1.options.length, sel2.options.length,
"Length should be same in select test " + aTestNumber);
is(aSel1.selectedIndex, sel2.selectedIndex,
"Selected index should be same in select test " + aTestNumber);
for (var i = 0; i < aSel1.options.length; ++i) {
is(aSel1.options[i].selected, sel2.options[i].selected,
"Options[" + i + "].selected should be the same in select test " +
aTestNumber);
is(aSel1.options[i].defaultSelected, sel2.options[i].defaultSelected,
"Options[" + i +
"].defaultSelected should be the same in select test " +
aTestNumber);
}
}
// In a single-select, setting an option selected should deselect an
// existing selected option.
var sel = document.createElement("select");
sel.appendChild(new Option());
is(sel.selectedIndex, 0, "First option should be selected");
assertSelected(sel.firstChild, false, true);
sel.appendChild(new Option());
is(sel.selectedIndex, 0, "First option should still be selected");
assertSelected(sel.firstChild, false, true);
assertSelected(sel.firstChild.nextSibling, false, false);
opt = new Option();
sel.appendChild(opt);
opt.defaultSelected = true;
assertSelected(sel.firstChild, false, false);
assertSelected(sel.firstChild.nextSibling, false, false);
assertSelected(opt, true, true);
is(opt, sel.firstChild.nextSibling.nextSibling, "What happened here?");
is(sel.options[0], sel.firstChild, "Unexpected option 0");
is(sel.options[1], sel.firstChild.nextSibling, "Unexpected option 1");
is(sel.options[2], opt, "Unexpected option 2");
is(sel.selectedIndex, 2, "Unexpected selectedIndex in select test 1");
assertSame(sel, "<select><option><option><option selected></select>", 1);
// Same, but with the option that gets set selected earlier in the select
sel = document.createElement("select");
sel.appendChild(new Option());
sel.appendChild(new Option());
opt = new Option();
opt.defaultSelected = true;
sel.appendChild(opt);
opt = new Option();
sel.options[0] = opt;
opt.defaultSelected = true;
assertSelected(sel.options[0], true, true);
assertSelected(sel.options[1], false, false);
assertSelected(sel.options[2], true, false);
is(sel.selectedIndex, 0, "Unexpected selectedIndex in select test 2");
// And now try unselecting options
sel = document.createElement("select");
sel.appendChild(new Option());
opt = new Option();
opt.defaultSelected = true;
sel.appendChild(opt);
sel.appendChild(new Option());
opt.defaultSelected = false;
assertSelected(sel.options[0], false, true);
assertSelected(sel.options[1], false, false);
assertSelected(sel.options[2], false, false);
is(sel.selectedIndex, 0, "Unexpected selectedIndex in select test 2");
</script>
</pre>
</body>
</html>

26
content/svg/content/src/nsSVGPathElement.cpp
View File

@ -434,32 +434,18 @@ nsSVGPathElement::BeforeSetAttr(PRInt32 aNamespaceID, nsIAtom* aName,
mSegments = nsnull;
}
nsSVGPathDataParserToInternal parser(&mPathData);
parser.Parse(*aValue);
if (aValue) {
nsSVGPathDataParserToInternal parser(&mPathData);
parser.Parse(*aValue);
} else {
mPathData.Clear();
}
}
return nsSVGPathElementBase::BeforeSetAttr(aNamespaceID, aName,
aValue, aNotify);
}
nsresult
nsSVGPathElement::UnsetAttr(PRInt32 aNamespaceID, nsIAtom* aName,
PRBool aNotify)
{
if (aNamespaceID == kNameSpaceID_None && aName == nsGkAtoms::d) {
if (mSegments) {
NS_REMOVE_SVGVALUE_OBSERVER(mSegments);
mSegments = nsnull;
}
mPathData.Clear();
return nsGenericElement::UnsetAttr(aNamespaceID, aName, aNotify);
}
return nsSVGPathElementBase::UnsetAttr(aNamespaceID, aName, aNotify);
}
NS_IMETHODIMP
nsSVGPathElement::DidModifySVGObservable(nsISVGValue* observable,
nsISVGValue::modificationType aModType)

2
content/svg/content/src/nsSVGPathElement.h
View File

@ -110,8 +110,6 @@ public:
virtual nsresult Clone(nsINodeInfo *aNodeInfo, nsINode **aResult) const;
virtual nsresult BeforeSetAttr(PRInt32 aNamespaceID, nsIAtom* aName,
const nsAString* aValue, PRBool aNotify);
virtual nsresult UnsetAttr(PRInt32 aNamespaceID, nsIAtom* aName,
PRBool aNotify);
protected:

5
content/xul/content/src/nsXULElement.cpp
View File

@ -1049,7 +1049,8 @@ nsXULElement::AfterSetAttr(PRInt32 aNamespaceID, nsIAtom* aName,
// Hide chrome if needed
if (aName == nsGkAtoms::hidechrome &&
mNodeInfo->Equals(nsGkAtoms::window)) {
mNodeInfo->Equals(nsGkAtoms::window) &&
aValue) {
HideWindowChrome(aValue && NS_LITERAL_STRING("true").Equals(*aValue));
}
@ -1225,6 +1226,8 @@ nsXULElement::FindAttrValueIn(PRInt32 aNameSpaceID,
nsresult
nsXULElement::UnsetAttr(PRInt32 aNameSpaceID, nsIAtom* aName, PRBool aNotify)
{
// This doesn't call BeforeSetAttr/AfterSetAttr for now.
NS_ASSERTION(nsnull != aName, "must have attribute name");
nsresult rv;

13
layout/reftests/bugs/395107-1-ref.html Normal file
View File

@ -0,0 +1,13 @@
<!DOCTYPE html>
<html>
<body>
<select id="a" size="5">
<option>Option 1</option>
<option selected>Option 2</option>
<option selected>Option 3</option>
<option selected>Option 4</option>
</select>
</body>
</html>

20
layout/reftests/bugs/395107-1.html Normal file
View File

@ -0,0 +1,20 @@
<!DOCTYPE html>
<html>
<body>
<select id="a" size="5">
<option>Option 1</option>
<option>Option 2</option>
<option>Option 3</option>
<option>Option 4</option>
</select>
<script>
document.body.offsetWidth;
var x=document.getElementById('a').options;
x[1].defaultSelected = true;
x[2].defaultSelected = true;
x[3].defaultSelected = true;
</script>
</body>
</html>

13
layout/reftests/bugs/395107-2-ref.html Normal file
View File

@ -0,0 +1,13 @@
<!DOCTYPE html>
<html>
<body>
<select id="a">
<option>Option 1</option>
<option selected>Option 2</option>
<option selected>Option 3</option>
<option selected>Option 4</option>
</select>
</body>
</html>

20
layout/reftests/bugs/395107-2.html Normal file
View File

@ -0,0 +1,20 @@
<!DOCTYPE html>
<html>
<body>
<select id="a">
<option>Option 1</option>
<option>Option 2</option>
<option>Option 3</option>
<option>Option 4</option>
</select>
<script>
document.body.offsetWidth;
var x=document.getElementById('a').options;
x[1].defaultSelected = true;
x[2].defaultSelected = true;
x[3].defaultSelected = true;
</script>
</body>
</html>

9
layout/reftests/bugs/395107-3-ref.html Normal file
View File

@ -0,0 +1,9 @@
<!DOCTYPE html>
<html>
<body>
<select size="2">
<option selected disabled>Option one</option>
<option>Option two</option>
</select>
</body>
</html>

10
layout/reftests/bugs/395107-3.html Normal file
View File

@ -0,0 +1,10 @@
<!DOCTYPE html>
<html>
<body onload='var elem1 = document.getElementById("one");
elem1.setAttribute("selected", "selected");'>
<select size="2">
<option id="one" disabled>Option one</option>
<option id="two" selected>Option two</option>
</select>
</body>
</html>

9
layout/reftests/bugs/395107-4-ref.html Normal file
View File

@ -0,0 +1,9 @@
<!DOCTYPE html>
<html>
<body>
<select>
<option selected disabled>Option one</option>
<option>Option two</option>
</select>
</body>
</html>

10
layout/reftests/bugs/395107-4.html Normal file
View File

@ -0,0 +1,10 @@
<!DOCTYPE html>
<html>
<body onload='var elem1 = document.getElementById("one");
elem1.setAttribute("selected", "selected");'>
<select>
<option id="one" disabled>Option one</option>
<option id="two" selected>Option two</option>
</select>
</body>
</html>

9
layout/reftests/bugs/395107-5-ref.html Normal file
View File

@ -0,0 +1,9 @@
<!DOCTYPE html>
<html>
<body>
<select size="2">
<option>Option one</option>
<option>Option two</option>
</select>
</body>
</html>

10
layout/reftests/bugs/395107-5.html Normal file
View File

@ -0,0 +1,10 @@
<!DOCTYPE html>
<html>
<body onload='var elem2 = document.getElementById("two");
elem2.removeAttribute("selected");'>
<select size="2">
<option id="one">Option one</option>
<option id="two" selected>Option two</option>
</select>
</body>
</html>

5
layout/reftests/bugs/reftest.list
View File

@ -441,6 +441,11 @@ fails == 386310-1d.html 386310-1-ref.html
== 394111-1.html about:blank # Really an assertion test rather than a rendering test
== 394534-1.html 394534-1-ref.html
== 394676-1.xhtml 394676-1-ref.xhtml
== 395107-1.html 395107-1-ref.html
== 395107-2.html 395107-2-ref.html
== 395107-3.html 395107-3-ref.html
== 395107-4.html 395107-4-ref.html
== 395107-5.html 395107-5-ref.html
== 395130-1.html 395130-1-ref.html
== 395130-2.html 395130-2-ref.html
== 395331-1.xml 395331-1-ref.xml