Closed Bug 472585 Opened 11 years ago Closed 11 years ago

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

Categories

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

defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla1.9.1

People

(Reporter: bent.mozilla, Assigned: bent.mozilla)

Details

(Keywords: fixed1.9.1)

Attachments

(1 file, 1 obsolete file)

Attached patch Patch, v1 (obsolete) — Splinter Review
'importScripts()' with no args throws an exception against spec.

It should just do nothing...

Trivial patch attached.
Flags: blocking1.9.1?
Attachment #355869 - Flags: superreview?(jonas)
Attachment #355869 - Flags: review?(jonas)
Attachment #355869 - Flags: superreview?(jonas)
Attachment #355869 - Flags: review?(jonas)
Attachment #355869 - Flags: review-
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.
Attached patch Patch, v1.1Splinter Review
Sigh.
Attachment #355869 - Attachment is obsolete: true
Attachment #355885 - Flags: superreview?(jonas)
Attachment #355885 - Flags: review?(jonas)
Attachment #355885 - Flags: superreview?(jonas)
Attachment #355885 - Flags: superreview+
Attachment #355885 - Flags: review?(jonas)
Attachment #355885 - Flags: review+
Comment on attachment 355885 [details] [diff] [review]
Patch, v1.1

a=sicking trivial spec complience fix
Flags: blocking1.9.1?
Assignee: nobody → bent.mozilla
Pushed changeset 3c0b74a013d7 to mozilla-central.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Pushed changeset 23587703248a to mozilla-1.9.1.
Keywords: fixed1.9.1
Component: DOM: Mozilla Extensions → DOM
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.