827035 - Etherpad won't load in 20.0a1 (2013-01-05)

Status: RESOLVED FIXED

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

Description

[Ravi Pina [:ravi]]

All etherpads display "Loading..." starting in 20.0a1 (2013-01-05).

Built from http://hg.mozilla.org/mozilla-central/rev/d8ca3e1c469e

Comment 1

[Ravi Pina [:ravi]]

This works in 20.0a1 (2013-01-04).

Built from http://hg.mozilla.org/mozilla-central/rev/801ba75ac563

Comment 2

[Richard Newman [:rnewman]]

Error Console output?

Comment 3

[:Gavin Sharp [email: [e-mail]]]

No errors in the console (I can reproduce on Mac). I'm bisecting.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

The first bad revision is:
changeset:   117473:39b595bc7d78
user:        Boris Zbarsky <[e-mail]>
date:        Thu Jan 03 14:02:36 2013 -0500
summary:     Bug 810644 part 1.  Switch setTimeout and setInterval to using WebIDL callback functions.  r=smaug, sr=peterv

Comment 5

[Boris Zbarsky [:bzbarsky]]

When I load that etherpad, this code in CallbackFunction::CallSetup::CallSetup gets hit:

  if (NS_FAILED(rv)) {
    // Security check failed.  We're done here.
    return;
  }

and we take the early return.  If I comment out that early return, everything works as far as I can tell.

Looking into why the security check fails.

Comment 6

[Boris Zbarsky [:bzbarsky]]

So the reason the security check fails is that the function being run is from a designMode document, but being set as a timeout on some other window.  What used to happen is that we did this "can execute scripts" check on the docshell of the window setTimeout was called on, not on the window of the actual function.

But for WebIDL callbacks we decided it makes a lot more sense to check the window of the actual function in cases like this.

so we land in nsDocShell::GetCanExecuteScripts and discover script is disabled.  That's not surprising, since this is editable stuff.  So we detect that case, and call GetJsAndPluginsDisabled on the editing session.  And this returns true, and then we hit this comment in docshell:

                                  // We have a docshell which has been explicitly set
                                  // to design mode, so we disallow scripts.

so we disallow the call.

Comment 7

[Boris Zbarsky [:bzbarsky]]

One option is to just give up and check for script being enabled or disabled in the document trying to call the callback, not in the document the callback comes from, in all cases.  That would match the behavior CallEventHandler used to have, pretty much.  I think.

Another option is to special-case the callbacks from setTimeout/setInterval somehow.

An interesting question for that second option is whether setting an on* attribute or adding an event listener to a designMode document from outside that document (so the reverse of the situation etherpad is in) should fire or not.  With the current WebIDL approach, it would, but if we take option 1 above I think it would not...

Comment 8

[Boris Zbarsky [:bzbarsky]]

On the gripping hand, if script is turned off by the user, should event listeners added to web pages by chrome work?  With the current WebIDL setup they would, I think, but with the setup we used to have, and with "one option" from comment 6, they would not.

Comment 9

[Boris Zbarsky [:bzbarsky]]

One more thought.  Right now we use the jscontext/window/document to decide two things, basically:

1)  Which window's onerror handler (and general error reporting mechanisms like the web
    console) to trigger on exceptions.
2)  Whether to run the function at all, based on whether script is enabled for that
    window/document.

Before bug 810644 we used the window setTimeout was called on for both of these.  Now we use the window the function comes from for both of these.  Per current html5 spec, our new behavior for #1 is correct, but that spec doesn't match our old behavior, or that of WebKit or Presto... but _does_ match IE and logic (see somewhat disjointed thread at <http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2012-November/037871.html>).

Still trying to figure out whether the spec says anything about #2.  It's possible that we may want to use different windows to determin #1 and #2 in some cases....

Comment 10

[Boris Zbarsky [:bzbarsky]]

One more thing: as far as I can tell, there is no mention of disabling scripting in designMode in the spec.  Do other UAs do that?

Comment 11

[(no longer active)]

(In reply to Boris Zbarsky (:bz) from comment #10)
> One more thing: as far as I can tell, there is no mention of disabling
> scripting in designMode in the spec.  Do other UAs do that?

IE used to at least in some old versions.  The reason that we do it is because nobody has fixed it, I'd take a patch which lifts this restriction in a heartbeat!

Comment 12

[Boris Zbarsky [:bzbarsky]]

It looks like the spec doesn't cover #2 at all.  I filed https://www.w3.org/Bugs/Public/show_bug.cgi?id=20578

As for disabling scripting in designMode, non-WebKit UAs certainly do it.  Filed https://www.w3.org/Bugs/Public/show_bug.cgi?id=20579

I also experimented a bit more in IE9.  This testcase:

test.html:

  <!DOCTYPE html>
  <body>
    <script>
      function f() {
        alert('aaa');
      }
      document.designMode = "on";
    </script>
  </body>

foo.html:

  <!DOCTYPE html>
  <iframe src="test.html"></iframe>
  <script>
    window.onload = function() {
      window.setTimeout(frames[0].f, 0);
    }
  </script>

does NOT run the function in IE9.  In quirks mode it runs the function, but it seems to not support setting designMode = "on" in quirks mode at all.

And unsurprisingly, the url in the URL field of this bug is broken in IE9 just like it's broken on trunk.

At which point this starts to look like an Etherpad bug to me.  Are any sites other than etherpad.mozilla.org affected?

Comment 13

[Boris Zbarsky [:bzbarsky]]

On the other hand, http://beta.etherpad.org/p/jHzrULI3cc works in IE9 but not on current trunk...  Of course it's possible they're doing some sort of UA sniffing now.

Comment 14

[Jesse Ruderman]

Enabling scripts in designMode documents (bug 765780) sounds reasonable to me.

Comment 15

[Boris Zbarsky [:bzbarsky]]

OK.  Sounds like I should back out bug 810644 until that's fixed?

Comment 16

[Boris Zbarsky [:bzbarsky]]

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/9944f44aff85

Comment 17

[Boris Zbarsky [:bzbarsky]]

I wonder whether I should have landed that on m-c instead to make the merge...

Comment 18

[Randell Jesup [:jesup] (needinfo me)]

probably... :-)  Save you an uplift request!

Comment 19

[Boris Zbarsky [:bzbarsky]]

Turns out we're doing an inbound merge before uplift after all.

But in any case, I also pushed to m-c: https://hg.mozilla.org/mozilla-central/rev/b5a78c274c09

Comment 21

[Boris Zbarsky [:bzbarsky]]

Fixed, I would assume, since I pushed to m-c.

Comment 23

[Manuela Muntean [Away]]

Verified with Firefox 20 beta 1, on a Mac OSX 10.7.5 machine.

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:20.0) Gecko/20100101 Firefox/20.0
Build ID: 20130220104816

The etherpad is displayed properly.