New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

start to fix let statements to make vimp compatible with FF 35.0a1 #40

Merged
merged 3 commits into from Oct 1, 2014

Conversation

Projects
None yet
5 participants
@Segaja
Contributor

Segaja commented Sep 17, 2014

start fixing issues that came up with FF bug https://bugzilla.mozilla.org/show_bug.cgi?id=1001090

@@ -1466,7 +1466,7 @@ const CommandLine = Module("commandline", {
function (args) {
var str = args.literalArg;
let str = CommandLine.echoArgumentToString(str, true);

This comment has been minimized.

@gkatsev

gkatsev Sep 17, 2014

Member

better to just change the var up top to be let.

@crossroads1112

This comment has been minimized.

crossroads1112 commented Sep 19, 2014

This does indeed work, however I get this error when opening Firefox:

Sourcing file failed: ReferenceError: can't access let declaration 'arg' before initialization 
@gkatsev

This comment has been minimized.

Member

gkatsev commented Sep 19, 2014

@crossroads1112 probably another variable that needs to get fixed.

@Segaja

This comment has been minimized.

Contributor

Segaja commented Sep 20, 2014

@crossroads1112 do you have a file name and/or a line number for this error?

@crossroads1112

This comment has been minimized.

crossroads1112 commented Sep 20, 2014

@Segaja I don't. How should I go about debugging? The only thing I did was clone the repo and manually add the changes detailed in this pull request.

@Segaja

This comment has been minimized.

Contributor

Segaja commented Sep 20, 2014

I'm lost aswell. I fixed the startup errors because the startup log lists them with file name and sometimes event line numbers, but I'm lost of no details are given. Maybe someone who knows the code better can have a look.

@crossroads1112

This comment has been minimized.

crossroads1112 commented Sep 20, 2014

Regardless, it's a good workaround. Vimperator works at least

@caisui

This comment has been minimized.

Contributor

caisui commented Sep 21, 2014

i fixed some another variable .

diff --git a/common/content/commands.js b/common/content/commands.js
index 89ff540..07d3667 100644
--- a/common/content/commands.js
+++ b/common/content/commands.js
@@ -548,6 +548,8 @@ const Commands = Module("commands", {
         var onlyArgumentsRemaining = allowUnknownOptions || (options.length == 0 && subCommands.length == 0) || false; // after a -- has been found
         var arg = null;
         var count = 0; // the length of the argument
+        var quote = null;
+        var error = null;
         var i = 0;
         var completeOpts;

@@ -692,7 +694,7 @@ const Commands = Module("commands", {
             }

             // if not an option, treat this token as an argument
-            let [count, arg, quote, error] = getNextArg(sub);
+            [count, arg, quote, error] = getNextArg(sub);
             liberator.assert(!error, error);

             if (complete) {
diff --git a/common/content/options.js b/common/content/options.js
index 78417ff..b52ce21 100644
--- a/common/content/options.js
+++ b/common/content/options.js
@@ -123,11 +123,11 @@ const Option = Class("Option", {
         else
             scope = this.scope;

+        let value;
         // Options with a custom getter are always responsible for returning a meaningful value
         if (this.getter)
             return liberator.trapErrors(this.getter, this, value);

-        let value;
         if (liberator.has("tabs") && (scope & Option.SCOPE_LOCAL))
             value = tabs.options[this.name];
         if ((scope & Option.SCOPE_GLOBAL) && (value == undefined))
@crossroads1112

This comment has been minimized.

crossroads1112 commented Sep 21, 2014

@caisui That fixed the startup errors

@Segaja

This comment has been minimized.

Contributor

Segaja commented Sep 21, 2014

Thanks @caisui . I will add these changes to my PR later today.

@crossroads1112

This comment has been minimized.

crossroads1112 commented Sep 21, 2014

Not sure if this is related but on when trying to use :back on some websites I get the following error:

[Exception... "Component returned failure code: 0x805e0006 [nsIWebNavigation.gotoIndex]" nsresult: "0x805e0006 (<unknown>)" location: "JS...

Unfortunately that is all it shows me. Is there a log I can look at to get the full error?

EDIT: No discernible pattern as to which webpages this happens on

@gkatsev

This comment has been minimized.

Member

gkatsev commented Sep 21, 2014

@crossroads1112 check out :messages

@crossroads1112

This comment has been minimized.

crossroads1112 commented Sep 21, 2014

Thanks @gkatsev

The error is as follows:

[Exception... "Component returned failure code: 0x805e0006 [nsIWebNavigation.gotoIndex]" nsresult: "0x805e0006 (<unknown>)" location: "JS frame :: chrome://liberator/content/liberator-overlay.js -> liberator://template/chrome://liberator/content/history.js :: stepTo :: line 65" data: no
@Segaja

This comment has been minimized.

Contributor

Segaja commented Sep 23, 2014

There is some part of the error message missing at the end.

For now the line in the code looks ok: https://github.com/vimperator/vimperator-labs/blob/master/common/content/history.js#L56-L67
and the function exists: https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIWebNavigation#gotoIndex%28%29

If you can show us the end of the error message maybe it will help.

@crossroads1112

This comment has been minimized.

crossroads1112 commented Sep 23, 2014

That was the end of the error message.

@maxauthority

This comment has been minimized.

Member

maxauthority commented Oct 1, 2014

Sorry, I only saw this thread now. Shall I still pull the request? Does it seem to work fine (apart the maybe unrelated :back problem)?


BTW: Some more information, which Mozilla sent to me:

[Action Required] Firefox 35 compatibility problems with your add-on

Hello,

As part of recent ES6 compliance efforts, the semantics of the let keyword have changed[1] in a non-backwards-compatible way which affects one or more of your add-ons. In particular:

  1. Variables may no longer be declared more than once in the same scope using the let keyword, and may not be redeclared using the let keyword if they have already been declared using var, or as a function argument, within the same lexical scope.

This means that, as of Firefox 35, none of the following are
acceptable:

function f(foo) {
let foo;
}

{
let foo;
let foo = y;
}

{
let foo;
var foo;
}

  1. Variables declared using the let keyword may no longer be referenced before the let statement which binds them has executed. This means that the following will no longer work:

    {
    x = 4;
    let x;
    }

    switch (2) {
    case 1:
    let x = 'foo';
    case 2:
    x = 'bar';
    }

You are receiving this email because the following add-on fall awry of the first class of new restrictions in the following files. Note that the second class of errors are only detectable during runtime, and may exist in other files or add-ons.

https://addons.mozilla.org/addon/vimperator/

common/content/commandline.js

If you need further assistance with these issues, please visit the add-ons forums https://forums.mozilla.org/addons, the #extdev IRC channel irc://irc.mozilla.org/extdev, or Stack Overflow http://stackoverflow.com/

Thanks,
The Mozilla Add-ons Team

[1] https://bugzil.la/1001090

So this will probably fix all that?

@crossroads1112

This comment has been minimized.

crossroads1112 commented Oct 1, 2014

Yes, you still should. Even if the back problem isn't unrelated, this pull request still fixes most of it

maxauthority added a commit that referenced this pull request Oct 1, 2014

Merge pull request #40 from Segaja/fix_let_statements
start to fix let statements to make vimp compatible with FF 35.0a1

@maxauthority maxauthority merged commit a374ceb into vimperator:master Oct 1, 2014

@gkatsev gkatsev referenced this pull request Oct 23, 2014

Closed

Updating the HACKING file #74

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment