472585 - Workers: 'importScripts()' with no args throws an exception against spec

Status: RESOLVED FIXED

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

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v1

'importScripts()' with no args throws an exception against spec.

It should just do nothing...

Trivial patch attached.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 355869 [details] [diff] [review]
Patch, v1

>@@ -231,22 +231,33 @@ nsDOMWorkerFunctions::LoadScripts(JSCont
>     }
> 
>     JSString* str = JS_ValueToString(aCx, val);
>     if (!str) {
>       JS_ReportError(aCx, "Couldn't convert argument %d to a string", index);
>       return JS_FALSE;
>     }
> 
>+    size_t length = JS_GetStringLength(str);
>+    if (!length) {
>+      // Don't bother with empty strings.
>+      continue;
>+    }
>+
>     nsString* newURL = urls.AppendElement();
>     NS_ASSERTION(newURL, "Shouldn't fail if SetCapacity succeeded above!");
> 
>     newURL->Assign(nsDependentJSString(str));
>   }
> 
>+  // Check to make sure we actually have some urls to resolve.
>+  if (!urls.Length()) {
>+    return JS_TRUE;
>+  }
>+
>   nsRefPtr<nsDOMWorkerScriptLoader> loader =
>     new nsDOMWorkerScriptLoader(worker);
>   if (!loader) {
>     JS_ReportOutOfMemory(aCx);
>     return JS_FALSE;
>   }
> 
>   nsresult rv = worker->AddFeature(loader, aCx);

Actually, empty-string is a valid url and should be resolved and loaded like anything else, so these two changes are not needed.

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v1.1

Sigh.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 355885 [details] [diff] [review]
Patch, v1.1

a=sicking trivial spec complience fix

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Pushed changeset 3c0b74a013d7 to mozilla-central.

Comment 5

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Pushed changeset 23587703248a to mozilla-1.9.1.