Closed Bug 511823 Opened 12 years ago Closed 12 years ago

Port Bug 494543 [Can't add items to Dell.com shopping cart] to SeaMonkey

Categories

(SeaMonkey :: Session Restore, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: misak.bugzilla, Assigned: misak.bugzilla)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch patch (obsolete) — Splinter Review
From parent bug description:

Using the url above, wait for the components animation to finish building,
click 'add to shopping cart' next to 'add to wishlist'. Notice nothing happens.
Mouseover the 'cart' text at the top of the page, you'll see 'your cart is
empty'.

Error console produces:
Error: uncaught exception: [Exception... "Security error" code: "1000"
nsresult: "0x805303e8 (NS_ERROR_DOM_SECURITY_ERR)" location:
"http://i.dell.com/images/global/js/s_code_dell_vh19_3.js Line: 647"]

Regression range:
4/10 - works
4/11 - broken

JIT is not a factor since I permanently disabled it. Tested with cookies & 3rd
party cookies on.

Reproducible: Always

URL is not working now, but here is another way to reproduce:

Try this:

javascript:try{sessionStorage;}catch(e){alert(e);}void(0);

That is supposed to do nothing. Seemingly every page on dell.com throws the
security error.
Attachment #395780 - Flags: superreview?(neil)
Attachment #395780 - Flags: review?(neil)
Comment on attachment 395780 [details] [diff] [review]
patch

>+    aDocShell.QueryInterface(Components.interfaces.nsIDocShell_MOZILLA_1_9_1_SessionStorage);
If I understand correctly, this is only a problem on the 1.9.1 branch, since on trunk the docShell already has the correct method? If so, I think we should #ifdef MOZILLA_1_9_1_BRANCH here.

>+        storage = aDocShell.getSessionStorageForPrincipal(principal, false);
>+        if (storage)
>+          storageItemCount = storage.length;
>         let docShell_191 = aDocShell.QueryInterface(Components.interfaces.nsIDocShell_MOZILLA_1_9_1_SessionStorage);
>         storage = docShell_191.getSessionStorageForURI(uri);
>         storageItemCount = storage.length;
That doesn't look right... mixed up with another patch perhaps?
That comes from parent bug 511635 patch, waiting for your review too ;)
Patch on top of patch for bug 511635, which got r+ and sr+, will land shortly.
Attachment #395780 - Attachment is obsolete: true
Attachment #400711 - Flags: superreview?(neil)
Attachment #400711 - Flags: review?(neil)
Attachment #395780 - Flags: superreview?(neil)
Attachment #395780 - Flags: review?(neil)
Comment on attachment 400711 [details] [diff] [review]
universal patch, uses #ifdef for 1.9.1 branch

> 
>+#ifdef MOZILLA_1_9_1_BRANCH
>+    aDocShell.QueryInterface(Components.interfaces.nsIDocShell_MOZILLA_1_9_1_SessionStorage);
>+#endif
Nit: put this before the blank line.

> #ifdef MOZILLA_1_9_1_BRANCH
>         let docShell_191 = aDocShell.QueryInterface(Components.interfaces.nsIDocShell_MOZILLA_1_9_1_SessionStorage);
>         storage = docShell_191.getSessionStorageForURI(uri);
Nit: this is relative to the wrong patch ;-)
In fact we don't need this since we already did aDocShell.QueryInterface above.
I think it's better doing the QueryInterface near the start instead.
Can we improve the other aDocShell.QueryInterface too?
Attached patch nits fixedSplinter Review
Attachment #400711 - Attachment is obsolete: true
Attachment #400731 - Flags: superreview?(neil)
Attachment #400731 - Flags: review?(neil)
Attachment #400711 - Flags: superreview?(neil)
Attachment #400711 - Flags: review?(neil)
Comment on attachment 400731 [details] [diff] [review]
nits fixed

>-    for (let url in aStorageData) {
>-      let uri = ioService.newURI(url, null, null);
> #ifdef MOZILLA_1_9_1_BRANCH
>       aDocShell.QueryInterface(Components.interfaces.nsIDocShell_MOZILLA_1_9_1_SessionStorage);
> #endif
>+    for (let url in aStorageData) {
>+      let uri = ioService.newURI(url, null, null);
Nit: indentation of aDocShell.QueryInterface needs to change.
Attachment #400731 - Flags: superreview?(neil)
Attachment #400731 - Flags: superreview+
Attachment #400731 - Flags: review?(neil)
Attachment #400731 - Flags: review+
fix for this bug will be checked in soon as combined patch for bug 511635
Patch checked in:

http://hg.mozilla.org/comm-central/rev/3eb3ed8481ed
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.