Closed Bug 896947 Opened 11 years ago Closed 10 years ago

Use asynchronous version of setCharsetForURI and getCharsetForURI in getShortcutOrURI and other places

Categories

(SeaMonkey :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.31

People

(Reporter: philip.chee, Assigned: neil)

References

(Depends on 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

See:
Bug 834543 - Add asynchronous version of setCharsetForURI and getCharsetForURI.
Bug 834543 - Part 2: Update use of setCharsetForURI.
Bug 834543 - Part 3: Update use of getCharsetForURI.
Bug 846635 - Use asynchronous getCharsetForURI in getShortcutOrURI.
See:
Bug 854925 - Remove SetCharsetForURI and GetCharsetForURI from nsINavHistoryService.
Attached patch WIP (obsolete) — Splinter Review
This is a sort of -w diff but the lines have been reindented to match the target file so it can't be applied to the tree.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attached patch Possible patch (obsolete) — Splinter Review
(Previous patch turned out to have some typos too, but it's still worth a read to get the gist of the -w version.)
Attachment #8463415 - Flags: review?(iann_bugzilla)
Attachment #8463415 - Flags: feedback?(philip.chee)
Fri Aug 01 2014 04:49:34
Error: A promise chain failed to handle a rejection. Did you forget to '.catch', or did you forget to 'return'?
See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise

Date: Fri Aug 01 2014 04:49:24 GMT+0800 (Malay Peninsula Standard Time)
Full Message: ReferenceError: uri is not defined
Source file: chrome://navigator/content/navigator.js
Line: 1813
Source code:

function handleDroppedLink(event, url, name)
{
  promiseShortcutOrURI(url).then(([url, postData]) => {
    if (uri) <-- typo
      loadURI(uri, null, postData, false);
  });
Comment on attachment 8463415 [details] [diff] [review]
Possible patch

>+++ b/suite/browser/navigator.js
> function handleDroppedLink(event, url, name)
> {
>+  promiseShortcutOrURI(url).then(([url, postData]) => {
>+    if (uri)
>+      loadURI(uri, null, postData, false);
>+  });
Where has uri come from?

>+++ b/suite/browser/navigatorDD.js

>   onDrop: function (aEvent)
>   {
>     var url = Services.droppedLinkHandler.dropLink(aEvent, {});
>+    promiseShortcutOrURI(url).then(([url, postData]) => {
>+      if (uri)
>+        loadURI(uri, null, postData, false);
>+    });
Again, more cut and paste issues?

>+++ b/suite/browser/tabbrowser.xml
>               }
>-            }
>+            });
Not sure where this change comes from.
>           ]]>
>         </body>
>       </method>

r- for the moment
Attachment #8463415 - Flags: review?(iann_bugzilla) → review-
(In reply to Ian Neal from comment #5)
> (From update of attachment 8463415 [details] [diff] [review])
> > function handleDroppedLink(event, url, name)
> > {
> >-  var postData = { };
> >-  var uri = getShortcutOrURI(url, postData);
> >-  if (uri)
> >-    loadURI(uri, null, postData.value, false);
> >+  promiseShortcutOrURI(url).then(([url, postData]) => {
> >+    if (uri)
> >+      loadURI(uri, null, postData, false);
> >+  });
> Where has uri come from?
uri was always there, but I typoed the name in the then clause.

> >   onDrop: function (aEvent)
> >   {
> >     var url = Services.droppedLinkHandler.dropLink(aEvent, {});
> >+    promiseShortcutOrURI(url).then(([url, postData]) => {
> >+      if (uri)
> >+        loadURI(uri, null, postData, false);
> >+    });
> Again, more cut and paste issues?
That one probably was cut and paste, yes. Sorry about that.

> >-            }
> >+            });
> >           ]]>
> >         </body>
> >       </method>
> Not sure where this change comes from.
There's a promiseShortcutOrURI(url).then(([url]) => { 29 lines earlier.
Attached patch Fixed patchSplinter Review
Attachment #8463413 - Attachment is obsolete: true
Attachment #8463415 - Attachment is obsolete: true
Attachment #8463415 - Flags: feedback?(philip.chee)
Attachment #8466803 - Flags: review?(iann_bugzilla)
> -            null, null, postData.value, true, isUTF8);
> +            null, null, postData, true, isUTF8);
I must really remember to file a bug to remove isUTF8 from SeaMonkey.
Comment on attachment 8466803 [details] [diff] [review]
Fixed patch

proforma f=me
Attachment #8466803 - Flags: feedback+
Attachment #8466803 - Flags: review?(iann_bugzilla) → review+
a=me for CLOSED TREE (and c-a/c-b if required)
Pushed comm-central changeset dc23a5a6112e.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.31
Blocks: 1148191
Blocks: 1148415
Depends on: 1174466
You need to log in before you can comment on or make changes to this bug.