Closed Bug 1073912 Opened 5 years ago Closed 5 years ago

octal literals and octal escape sequences are deprecated: components/XULStore.js

Categories

(Core :: XUL, defect, trivial)

defect
Not set
trivial

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: ishikawa, Assigned: ishikawa)

References

Details

Attachments

(1 file, 1 obsolete file)

During testing, I noticed that the following message
was printed many times to the invoking console.    

JavaScript strict warning: file:///REF-OBJ-DIR/objdir-tb3/dist/bin/components/XULStore.js, line 147: SyntaxError: octal literals and octal escape sequences are deprecated

Despite Bug 928725, I think 0o prefix still needs a time to be accepted widely obviously (from developers even by this bug, mind you), I suggest a patch to use ParseInt (..., 8) to shut up the warning completely.

We need every trick to reduce the warning in the test output (noise and signal ratio is very bad.)

If only many developers form a habit of checking the console log  test suite runs with DEBUG version of binaries every month,
and see if their new code or legacy code may have started to producing new warnings or not.


TIA
Here is the patch.

(In reply to ISHIKAWA, Chiaki from comment #0)

> If only many developers form a habit of checking the console log  test suite
> runs with DEBUG version of binaries every month,
> and see if their new code or legacy code may have started to producing new
> warnings or not.

This was meant NOT to this bugzilla entry alone, but to general
mozilla developer community as a whole.
Blocks: 826732
Comment on attachment 8496550 [details] [diff] [review]
Replaced literal octal string(s) with ParseInt() call(s).

Review of attachment 8496550 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/xulstore/XULStore.js
@@ +143,5 @@
>    },
>  
>    readFile: function() {
>      const MODE_RDONLY = 0x01;
> +    const FILE_PERMS  = ParseInt("0600", 8);

I think you can use ES6 syntax 0o600 rather than parseInt.
(In reply to Marco Bonardo [::mak] (needinfo? me) from comment #2)
> 
> I think you can use ES6 syntax 0o600 rather than parseInt.

Thank you for the comment.

I was a little uncomfortable to use the 0o notation since ES6 is relatively new (?: I am not
familiar with ES6 standard process), and
if "0o" is the preferred way in the mozilla community today, I have no problem with it.

Any thought about compatibility issues?
(Not within the firefox/thunderbird internal usage only, but would it be OK to
encourage the readers of the code to mimic the usage and still they don't run into
compatibility issues? Now that I think about it, Google(chrome), Microsoft(IE6), etc. are
behind ES6 efforts, maybe not much. )

I will create the modified patch with 0o prefix if there is an agreement on this.

I am putting Gavin Sharp, one of the peers of Toolkit (XUL Toolkit) in
http://www-archive.mozilla.org/owners.html
as the reviewer who needs to ack the eventual patch.

Let the peer(s) decide.

For a discussion of 0o, maybe the almost one year old
Bug 928725 (towards the end)
might be interesting. I am afraid that we may need to educate
people on the use of 0o if it is now supported sold and widely.

TIA
Assignee: nobody → ishikawa
Attachment #8496550 - Flags: review?(gavin.sharp)
Comment on attachment 8496550 [details] [diff] [review]
Replaced literal octal string(s) with ParseInt() call(s).

I think you mean "parseInt"... But I believe you can use 0o600 instead? I think that would be preferred. Thanks for the patch!
Attachment #8496550 - Flags: review?(gavin.sharp) → review-
Oh, sorry, I missed the earlier comments. Yes, the 0o600 style is preferred. And by the way, Marco's also a toolkit peer! :) https://wiki.mozilla.org/Modules/Toolkit is the up-to-date page for that.
See Also: → 1073955
(I have four similar bugzilla entries/patches. Here is a template
response.)

Thank you for the review.

I am uploading a new patch with that uses "0o" prefix for
octal literal. It seems "0o..." notation is preferred over
parseInt(..., 8).

Regarding the misspelling of "ParseInt" for "parseint",
I thought initially people were referring to the misspelling
in the description field of my patch.

I said "Whoa!" when I realized that the code itself had this misspelling(!)

SURPRISE: I had applied the patch to my source tree, and had run the
|make mozmill| test, and JS and mozmill test frame never printed a
single complaint about this (!!!)

That is why I did not realize the misspelling in the code portion of
the patch.

I carefully checked the log files and found no mention of ParseInt at
all. Also, the number of failed errors were not that much
different before and after the patch.
(Because of timing-sensitive errors that happen during shutdown (bugs), etc.,
the number of failed errors may differ from one run to the other. I
did not see any major errors possibly due to the "ParseInt"
misspellings.)
So, I think JS interpreter is doing something without aborting the 
execution.

What gives? I think this merits a new bugzilla entry...

Anyway, the simple "Use 0o prefix for octal literal" patch is
uploaded.

TIA
Attachment #8497243 - Flags: review?(gavin.sharp)
Attachment #8497243 - Flags: review?(gavin.sharp) → review+
(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #5)
> Oh, sorry, I missed the earlier comments. Yes, the 0o600 style is preferred.
> And by the way, Marco's also a toolkit peer! :)
> https://wiki.mozilla.org/Modules/Toolkit is the up-to-date page for that.

Thank you for the review+.
I will put in the checkin_needed keyword.

I did not mean to slight Marco, sorry about my mixup.

It is just that Google showed the outdated page BEFORE the
up-to-date page when I searched for "mozilla module owner" :-(

TIA
Keywords: checkin-needed
Attachment #8496550 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/939cf8134947
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.