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

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Philip Chee]

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.

Comment 1

[[e-mail]]

See:
Bug 854925 - Remove SetCharsetForURI and GetCharsetForURI from nsINavHistoryService.

Comment 2

[[e-mail]]

Attachment: WIP

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.

Comment 3

[[e-mail]]

Attachment: Possible patch

(Previous patch turned out to have some typos too, but it's still worth a read to get the gist of the -w version.)

Comment 4

[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 5

[Ian Neal]

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

Comment 6

[[e-mail]]

(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.

Comment 7

[[e-mail]]

Attachment: Fixed patch

Comment 8

[Philip Chee]

> -            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 9

[Philip Chee]

Comment on attachment 8466803 [details] [diff] [review]
Fixed patch

proforma f=me

Comment 10

[Ian Neal]

a=me for CLOSED TREE (and c-a/c-b if required)

Comment 11

[[e-mail]]

Pushed comm-central changeset dc23a5a6112e.