666985 - Accessing quotes on morningstar.com does not work in FF5, but does in FF3/FF4

Status: VERIFIED FIXED

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

Description

[Terry R.]

User-Agent:       Mozilla/5.0 (Windows NT 5.1) AppleWebKit/534.30 (KHTML, like Gecko) Chrome/12.0.742.100 Safari/534.30
Build Identifier: Firefox 5.0

When a stock symbol is entered into the Quote box at the top of the page and "Quote" is clicked, a Javascript page is shown with, "javascript:HeaderBox.trySubmit();" and "false" on the page.  If the symbol is added to the URL (ex: delete IBM and enter C), the page works.


Reproducible: Always

Steps to Reproduce:
1. Go to URL
2. Enter stock symbol to the left of "Quote"
3. Click "Quote"

Comment 1

[(mostly gone) XtC4UaLL [:xtc4uall]]

Reproduced against Mozilla/5.0 (Windows NT 5.1; rv:7.0a1) Gecko/20110624 Firefox/7.0a1 ID:20110624030724

Last good nightly: 2011-04-11 First bad nightly: 2011-04-12
Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=09b605eb3e0d&tochange=a174b86200d6

Last good nightly: 2011-04-12 First bad nightly: 2011-04-13
Pushlog: http://hg.mozilla.org/tracemonkey/pushloghtml?fromchange=eee087772f45&tochange=cd5910633dd8

Comment 2

[Boris Zbarsky [:bzbarsky]]

hg bisect says:

The first bad revision is:
changeset:   67818:74e91dcb802d
user:        David Zbarsky <[e-mail]>
date:        Mon Apr 11 18:56:37 2011 +0200
summary:     Bug 583514 - implement click and accesskey for all HTML elements r=smaug

This site confuses me a bit.  The "Quote" button is actually an <a>:

  <a class="hqt_button" href="javascript:HeaderBox.trySubmit();"></a>

In particular, note that javascript: URI.  Looking at the HeaderBox.trySubmit function, it looks like this:

function () {
    if (SelectedIndex >= 0 && SelectedIndex < NumShownEntries) {
        replaceCurrentKey(SelectedIndex);
        if (self.EntryChosenCallback != null) {
            var entry = ShownEntries[SelectedIndex];
            if (typeof self.EntryChosenCallback == "string") {
                var func_str = self.EntryChosenCallback + "(entry)";
                delayedHideDropDown();
                try {
                    eval(func_str);
                } catch (e) {}
            } else {
                try {
                    self.EntryChosenCallback(entry);
                } catch (e) {}
            }
            delayedHideDropDown();
            return false;
        }
    } else {
        SelectedIndex = -1;
    }
    var quoteInput = document.getElementById(self.ACINPUT_ID);
    if (quoteInput != null) {
        var line = quoteInput.value;
        line = removeLastDelimiter(line);
        if (line != null) {
            var str = leftTrim(line);
            if (str != "") {
                commonSearch(QUOTE_SERVER, line);
            }
        }
    }
    delayedHideDropDown();
    return false;
}

Note that this always returns false....  And that should get converted to the sting "false" and loaded as a new document, which is exactly what happens on trunk.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Ah, looks like running trySubmit() on trunk just doesn't do a load, while in Fx4 it does.

In particular, we seem to land inside navigateWithReferrer in the JS file I'm about to attach (unpacked and deobfuscated from the site source), which looks like this:

    function navigateWithReferrer(url) {
        var fakeLink = document.createElement("a");
        if (document.createEvent) {
            location.href = url
        } else {
            fakeLink.href = url;
            document.body.appendChild(fakeLink);
            fakeLink.click()
        }
    }

we create fakeLink, then take the |else| branch of that if.  That triggers the link, but link clicks start loads async, so the stop from that load doesn't happen until after the js url has returned false and the page has gone away.  On the other hand, had we taken the location.href set path, we would have canceled the javascript: load due to starting the href load.

Now what confuses me is why document.createEvent tested false!

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Deobfuscated script

Comment 5

[Boris Zbarsky [:bzbarsky]]

Oh, I see what happened here.  That attached script is NOT the right script.  The right script (in obfuscated form) is <http://im.mstar.com/common/js/common_pack_0.0.4.js> while what I attached is from <http://im.mstar.com/common/js/common_pack_0.0.12.js>.  The latter loads first, so the 0.0.4 version overrides it, since it loads later.

The navigateWithReferrer in 0.0.4 looks like this:


    function navigateWithReferrer(url) {
        var fakeLink = document.createElement("a");
        if (typeof (fakeLink.click) == 'undefined') {
            location.href = url
        } else {
            fakeLink.href = url;
            document.body.appendChild(fakeLink);
            fakeLink.click()
        }
    }

which is why the click change affected it.

At this point, this looks like just a website bug: they updated their object-sniffing, but incorrectly deployed their script....

Comment 6

[Boris Zbarsky [:bzbarsky]]

On the other hand, looks like step 6 at http://www.whatwg.org/specs/web-apps/current-work/multipage/history.html#navigate says to cancel the existing javascript: load, and these steps are supposed to run sync under the activation of the <a> (whereas we run them async)....

I'm not sure yet what the security implications would be of running the loading algorithm sync from anchor activation.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Isn't this basically Bug 666604.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Ah, indeed.  I guess the click() is just a no-op as things stand.  With that trusted event check taken out, this bug is fixed.

Comment 9

[Kent]

Quotes on Morningstar started working for me yesterday using Firefox 5.0 after reinstalling it with a fresh download.

Does anyone know if Morningstar made a change?

Comment 10

[Boris Zbarsky [:bzbarsky]]

Yes, they did.  They're now loading common_pack_0.0.12.js twice instead of loading it once and then loading common_pack_0.0.4.js.  So now the code that runs is the code in comment 3, which works.

Comment 11

[Boris Zbarsky [:bzbarsky]]

This is fixed by bug 666604.

Comment 12

[Ioana (away)]

Verified as fixed on:
Mozilla/5.0 (Windows NT 6.1; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (X11; Linux i686; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Windows NT 5.1; rv:7.0) Gecko/20100101 Firefox/7.0
+ Aurora (Fx 8.0a2) & Nightly (Fx 9.0a1).

Verified this fix using the STR from the bug description. All the quotes worked fine.