8949 - Complex javascript program doesn't work

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Stephen P. Morse]

This should be consider more of an acceptance test than an individual bug report
because I'm sure it emcompasses many bugs, the bottom line of which is that
the program at this URL doesn't work.

The URL contains a javascript program that converts between dates in the Julian
(Gregorian?) calendar and the Jewish calendar.

Here are some of the problems.  For starters, the initial date displayed is
supposed to be the current date.  But instead it is coming up as Jan 1, 1900.
Second, changing any entry in any of the select lists is supposed to cause the
other select lists to be automatically updated but it is not -- you need to
bring down the original select list a second time in order to get the update (I
previously filed a bug report on that problem).  And, finally, the answers being
computed are not correct (they are not even close).  To see what the correct
conversions should be, run the same URL under any 4.x browser.

Comment 1

[brendan-obsolete]

Sounds like there's a DOM bug or two here -- Steve, what was the bug number for
the select onChange double-take problem?  We can make this bug a nonterminal
that depends on the "leaf" bugs (DOM, JS date, whatever they may be).

/be

Comment 2

[Eric Pollmann]

The DOM double-take bug == the DOM onChange bug == Bug 3322

Comment 3

[brendan-obsolete]

OK, I added 3322 to "Bugs that bug 8949 depends on" (anyone coulda done it).  If
there are other bugs split out after testing, please make them dependencies of
this bug.

/be

Comment 4

[rogerl (gone)]

I debugged a version of this code but couldn't find any JS errors in the
calculation - maybe it only seemed broken because the input/output behaviour is
so badly messed up.

Comment 5

[Mike McCabe]

Roger -

You're it :)

Could you take ownership of this bug, possibly closing it?

Comment 6

[rogerl (gone)]

I'm going to close this bug off since I'm convinced there's nothing pertaining
to JS engine bugs.

Comment 7

[Stephen P. Morse]

How can you close out a bug report just because you are "convinced there's
nothing pertaining to JS engine bugs."  Decide what it is pertaining to, and
reassign to the appropriate person.  If you don't know what it pertains to, then
assign it to choffman.  But don't close out a bug that is not fixed.

I'm reopening this report.

Comment 8

[cbegle]

can you provide a simplified test case that demonstrates the JavaScript bug?

Comment 9

[Stephen P. Morse]

It doesn't make sense to provide a simplified example here because, as I said in
the first line, this is more of an acceptance test than an individual bug
report.  There is probably more than one bug involved.  The bottom line is that
this site doesn't work.

There might not be any javascript bugs at all and that is why you should
reassign it if you feel that javascript is not the problem.

Comment 10

[cbegle]

Attachment: testcase

Comment 11

[cbegle]

simplified test case attached.  i poked the JS  a little, like in the
EnglishMonth frame.  JS knows what the right month should be, but the line that
sets the selected element doesn't seem to update the dom. this doesn't look
like a dupe of 3322 since you don't need onchange to see the bug.

Comment 12

[cbegle]

in the test case, the selected index should be 2.  it's 0 in apprunner.

Comment 13

[Eric Pollmann]

Thank you for narrowing this down.

The reason this doesn't work is because we implemented the DOM according to the
spec which says:

interface HTMLOptionElement : HTMLElement {
    readonly attribute  HTMLFormElement      form;
             attribute  boolean              defaultSelected;
    readonly attribute  DOMString            text;
             attribute  long                 index;
             attribute  boolean              disabled;
             attribute  DOMString            label;
--> readonly attribute  boolean              selected; <--
             attribute  DOMString            value;
};

You can change the selected option by poking the selectedIndex attribute of the
<SELECT> rather than by poking the selected attribute of the <OPTION>.

Looks like an invalid test case to me.

Comment 14

[leger]

Clearing Invalid resolution due to reopen.  But it sounds like a
Resolved/Invalid..yes?

Comment 15

[Stephen P. Morse]

OK, I can believe this.  By making the indicated change to the original URL
cited in this report, the page is indeed doing what it is supposed to do.  The
dates are being displayed correctly and the conversion is giving the correct
answers.

There is still the issue of the update not occuring automatically but that is
already covered in bug report 3322.

The only issue I have is one of compatibility.  Since this worked in 4.x in
spite of the mis-use of the "selected" attribute, won't we be breaking many
websites if we no longer allow this to work?

But I'm content to see this bug report closed out as invalid.

Comment 16

[brendan-obsolete]

Steve's right, we should not break level 0 DOM scripts.  This has worked since
2.0, IIRC.  Vidur, what was the DOM w.g. rationale for making selected readonly?

/be

Comment 17

[Dan203]

This page has a basic JS error that causes it to fail in NN4.x too!!! Line 118
sets the variable defective to +1, causing a runtime error.

defective = +1; //Improper JS syntax

should be...
defective = 1; or defective += 1; //Changing it to just = 1 made it work for NN4

Comment 18

[Stephen P. Morse]

This comes as a total surprise to me.  So javascript has a unary minus but not a
unary plus.  I just checked the javascript reference manual and you are correct.
But you must admit, this omission is very strange.  I wanted to have a trinary
variable which could take on the value -1, 0, or +1 so it was very natural to
code it as "defective = +1".

However, I don't see the runtime error you are describing, either in v4 or v5.
The javascript consule is clean in both cases.  What did you do to produce this
error, other than simply bringing up the page?

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I'm think the "readonly" on selected is an error in DOM1.  If selected is
readonly, then there is *no way* to manipulate option elements with multiple
set.  Furthermore, the DOM Errata page,
http://www.w3.org/DOM/updates/REC-DOM-Level-1-19981001-errata.html ,
updates the descriptions for selected and defaultSelected (as well as similar
errata for HTMLInputElement and HTMLTextAreaElement).  It doesn't give an
erratum for changing the IDL (as I think it should), but the prose description
for selected mentions "changing this attribute".  Here's the whole bit on
HTMLOptionElement (since there aren't any anchors in the errata):

Interface HTMLOptionElement

      The description of the defaultSelected attribute should read:

            Represents the value of the HTML selected attribute. The value of
this attribute does not change if the state of the corresponding form control in
an interactive user agent changes. Changing defaultSelected, however, resets the
state of the form control. See the selected attribute definition in HTML 4.0.

      The description of the selected attribute should read:

            Represents the current state of the corresponding form control in an
interactive user agent. Changing this attribute changes the state of the form
control, but does not change the value of the HTML selected attribute of the
element.

Comment 20

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I filed bug 12350 on issues with HTMLSelectElement::selected and
defaultSelected.  There were some other problems in addition to being selected
being readonly.  I'm marking it as a dependency.  (There could be other problems
here...)

Comment 21

[brendan-obsolete]

JavaScript has had unary + since ECMA edition 1, which was finalized in 1997
IIRC.  I think 4.5 was when we first shipped a fully compliant implementation.

/be

Comment 22

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

As far as I can tell from the discussion, there's no bug here other than the one
covered in 12350.  Can we close it?

Comment 23

[Stephen P. Morse]

Closing it is fine with me (see my comment of 8-2).

Comment 24

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Another one bites the dust.

Comment 25

[Prashant Desale]

Marking verified.

Comment 26

[vidur (gone)]

For anyone who cares. Bug 12350 has been fixed and the "readonly" qualifier for
the selected property was changed in the errata DOM Level 1.