292921 - netflix star hack doesn't load new stars image anymore

Status: RESOLVED FIXED

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

Description

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

Netflix (web pages available only to subscribers, unfortunately), allows rating
of movies.  It seems to have the image of stars inside an iframe and it loads a
new document inside the iframe when you click on the stars to show immediate
visual feedback on the rating, but only once it's actually happened successfully.

This regressed so that it only shows its "Saving..." in the little iframe with
stars and never shows the final result.

The regression occurred between Firefox builds 2005-01-29-07-trunk and
2005-01-31-23-trunk.  It works correctly in suite 2005-01-30-19-trunk .  This
gives a checkin query of:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=SeaMonkeyAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-01-30+16%3A00&maxdate=2005-01-31+23%3A59&cvsroot=%2Fcvsroot

Console output in a working suite build shows:
  Document http://www.netflix.com/SetRating?... loaded successfully
Console output in a broken suite build shows:
  Error loading URL http://www.netflix.com/SetRating?... : 804b0002

0x804b0002 seems to be NS_BINDING_ABORTED.

I'm not sure if I'll have time to reduce a testcase -- it looks quite
complicated.  But I can test backouts, and probably will shortly...

Comment 1

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

Reverting revision 1.656 of nsDocShell.cpp fixes this, so it was caused by bug
277092.

Comment 2

[Boris Zbarsky [:bzbarsky]]

So the problem is that this site does exactly what bug 277092 was trying to
"fix" -- sets a timeout to do some stuff, and then changes the location to a
page that returns 204 (so the document won't change).

I just tested IE and Opera, and neither one kills timeouts on location change. 
So I think our best course of action is to just back out bug 277092... I don't
see how we can keep that behavior without breaking netflix.

Comment 3

[Brendan Eich [:brendan]]

This all goes back to Netscape 2 (I was reminiscing on IRC, like Grandpa
Simpson; "back then we wore an onion on our belt -- that was the style at the
time").  The non-blocking (and non-reentrant, whee) i/o driven state machinery
there did not kill anything when you started a URL load.  Nor does starting a
load for Gecko, in general.

/be

Comment 4

[Boris Zbarsky [:bzbarsky]]

> Nor does starting a load for Gecko, in general.

That's not quite right, as it happens.  Starting a load in a docshell for Gecko
kills all existing network requests in the document loadgroup.  Starting a load
via setting location additionally kills all image animations and terminates the
parser (so if there's any data that's been received but not parsed yet it's
dropped).

Comment 5

[Brendan Eich [:brendan]]

bz: right, you said that on IRC -- thanks for reminding me.  Then I said "that's
not really what we should do (not what mosaic or netscape did, IIRC)" and you
tested Opera and another browser.

Are we just killing stuff prematurely all over the place?  If so, we should have
a general bug.

/be

Comment 6

[Boris Zbarsky [:bzbarsky]]

I think the killing of some things when location is changed was purposeful...
not sure what the purpose was.

Killing network requests on new document load start may also be purposeful; not
sure how our progress listeners react to that not happening.

Back to this bug, anyone see a better solution than just backing out bug 277092?
 If not, anyone want to a= the backout?  ;)

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Backout

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 182799 [details] [diff] [review]
Backout

sr+a=me, and maybe jst knows of a general bug on not canceling things when a
load is *started* (as opposed to when the first segment of data from that new
load arrives).

/be

Comment 9

[Johnny Stenback (:jst)]

Comment on attachment 182799 [details] [diff] [review]
Backout

r=jst. I'm not aware of a bug on early killing of requests etc...

Comment 10

[Boris Zbarsky [:bzbarsky]]

Fixed