553982 - myvidoop.com login broken on trunk

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Henri Sivonen (:hsivonen)]

Steps to reproduce:
 1) Make sure you have a myvidoop.com account.
 2) Go to https://myvidoop.com/login (old parser or HTML5 parser doesn't make a difference here)
 3) Enter your userid.
 4) Press the button.

Actual results:
A DHTML box comes up, but it's horizontally collapsed, so you don't see any useful content in the box and can't interact with it.

Expected results:
Expected the DHTML box to contain the next step of the login process (either a request form for browser activation code or a grid of 9 images depending on whether you already have the activation cookie).

Additional info:
Works in Firefox 3.5.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Can you give me a usable userid to reproduce this?  (Obviously I don't need a password.)

Comment 2

[Henri Sivonen (:hsivonen)]

The user id hsivonen takes you to the buggy page.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Interesting.  On trunk, I get the same behavior no matter what I put in the username field.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Regression range on m-c on Mac: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ff3496b1f6c7&tochange=bf0fdec8f43b

I have no idea why the imagelib changes would have affected this....

Comment 5

[Joe Drew (not getting mail)]

We need to figure out how decode-on-draw broke this.

Comment 6

[Bobby Holley (:bholley)]

I investigated this this afternoon. In a pre-decode-on-draw nightly, entering a bogus username and hitting submit sends your browser to a new page. In a post-decode-on-draw nightly, entering a bogus username and hitting submit opens up a broken and unreadable DHTML window.

In a nutshell, the site seems to be doing a bunch of really funky conditional stuff based on the loading of images and flash objects in pursuit of security. It's not clear to me how effective this is, because I don't understand what they're trying to do.

The relevant code is here (see the onsubmit handler):
https://myvidoop.com/js/vidsec_grid.js

I've sent an email to the parent company to see if I can get in touch with an engineer. Hopefully I'll hear back soon.

Comment 7

[Benjamin Stover (:stechz)]

Attachment: Test case

blank.php must have no characters in it.

Try http://stechz.com/scratch/testimage.php to see test case in action.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Standalone testcase, no php server needed

Comment 9

[Bobby Holley (:bholley)]

Figured this out.

Normally a bogus image (for example, <img src="foo.html">) will trigger onerror, because the first few bytes fail to sniff to a usable mimetype in OnDataAvailable(), which throws subsequently throws an error. However, If there are actually zero bytes of data, OnDataAvailable() never gets called, and we jump straight to OnStopRequest().

In 1.9.2, we checked in OnStopRequest to make sure that we had actually instantiated mImage:

http://hg.mozilla.org/releases/mozilla-1.9.2/file/09e35043fedb/modules/libpr0n/src/imgRequest.cpp#l847

However, this check got lost during the rewrite:
http://hg.mozilla.org/mozilla-central/file/f67e48bacc76/modules/libpr0n/src/imgRequest.cpp#l1011

I've written a one-line patch that adds the check. It fixes the myvidoop issue. I'm going to add in a mochitest and then put it up for review.

Comment 10

[Bobby Holley (:bholley)]

Attachment: patch v1

Added a patch with tests. Flagging joe for review.

pushed to try as 132d1e5a9ca1

Comment 11

[Boris Zbarsky [:bzbarsky]]

That test will pass if neither event fires.  It might be better to have the test structured by having a boolean defaulting false that the correct event will set to true and then in the page onload asserting the boolean is true.

Comment 12

[Bobby Holley (:bholley)]

Attachment: patch v2

Updated patch to address bz's concerns.

Comment 13

[Bobby Holley (:bholley)]

This looks green on try. I'll push it as soon as I get review from joe.

Comment 14

[Bobby Holley (:bholley)]

pushed to mozilla-central:

http://hg.mozilla.org/mozilla-central/rev/4e1d168e50cf

Resolving fixed.