Closed
Bug 1110166
Opened 10 years ago
Closed 10 years ago
Port |Bug 1102219 - Rename String.prototype.contains to String.prototype.includes| to comm-central
Categories
(Thunderbird :: General, defect)
Thunderbird
General
Tracking
(thunderbird39 unaffected, thunderbird40 fixed)
RESOLVED
FIXED
Thunderbird 40.0
Tracking | Status | |
---|---|---|
thunderbird39 | --- | unaffected |
thunderbird40 | --- | fixed |
People
(Reporter: clokep, Assigned: aryx)
References
Details
Attachments
(8 files)
4.94 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
7.56 KB,
patch
|
aleth
:
review+
|
Details | Diff | Splinter Review |
11.12 KB,
patch
|
iannbugzilla
:
review+
|
Details | Diff | Splinter Review |
50.95 KB,
patch
|
mkmelin
:
review+
|
Details | Diff | Splinter Review |
63.24 KB,
patch
|
mkmelin
:
review+
|
Details | Diff | Splinter Review |
2.02 KB,
patch
|
iannbugzilla
:
review+
|
Details | Diff | Splinter Review |
1.02 KB,
patch
|
iannbugzilla
:
review+
|
Details | Diff | Splinter Review |
3.08 KB,
patch
|
iannbugzilla
:
review+
|
Details | Diff | Splinter Review |
Bug 1102219 will bust us, it just landed on mozilla-inbound. I couldn't find a bug about porting this to TB/IB/SM. Figured Archaeopteryx might know what needs to be done for this...
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → archaeopteryx
Status: NEW → ASSIGNED
Comment 1•10 years ago
|
||
(In reply to Patrick Cloke [:clokep] from comment #0)
> Bug 1102219 will bust us, it just landed on mozilla-inbound.
No it won't. What will really break comm-central is Bug 1103588
Reporter | ||
Comment 2•10 years ago
|
||
(In reply to Philip Chee from comment #1)
> (In reply to Patrick Cloke [:clokep] from comment #0)
> > Bug 1102219 will bust us, it just landed on mozilla-inbound.
> No it won't. What will really break comm-central is Bug 1103588
Yeah, we discussed this on IRC. Not a big deal though, we still need to fix it.
Comment 3•10 years ago
|
||
Bug 1102219 jut landed on m-c again, and the tests are pretty much busted since we check for "errors" in the console, and now there is the deprecation warning.
"CONSOLE_MESSAGE: (warn) [JavaScript Warning: "String.prototype.contains() is deprecated and will be removed in a future release; use String.prototype.includes() instead" {file: "resource://testing-common/mailnews/imapd.js" line: 1427}]"
Error console says [stackFrame String.prototype.contains() is deprecated and will be removed in a future release; use String.prototype.includes() instead]
../../../../mailnews/resources/logHelper.js:_errorConsoleTunnel.observe:95
/opt/comm-central/src/mozilla/testing/xpcshell/head.js:_do_main:207
/opt/comm-central/src/mozilla/testing/xpcshell/head.js:_execute_test:513
-e:null:1
null:null:0
exiting test
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 5•10 years ago
|
||
Patches pushed to Try: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=12b350264325
Will request reviews tomorrow morning if they pass.
Assignee | ||
Comment 6•10 years ago
|
||
Attachment #8600278 -
Flags: review?(philipp)
Assignee | ||
Comment 7•10 years ago
|
||
Attachment #8600279 -
Flags: review?(aleth)
Assignee | ||
Comment 8•10 years ago
|
||
Attachment #8600280 -
Flags: review?(iann_bugzilla)
Assignee | ||
Comment 9•10 years ago
|
||
Attachment #8600281 -
Flags: review?(mkmelin+mozilla)
Assignee | ||
Comment 10•10 years ago
|
||
Attachment #8600282 -
Flags: review?(mkmelin+mozilla)
Assignee | ||
Comment 11•10 years ago
|
||
Attachment #8600283 -
Flags: review?(neil)
Assignee | ||
Comment 12•10 years ago
|
||
Attachment #8600284 -
Flags: review?(neil)
Comment 13•10 years ago
|
||
Comment on attachment 8600279 [details] [diff] [review]
chat, v1
Review of attachment 8600279 [details] [diff] [review]:
-----------------------------------------------------------------
::: chat/protocols/irc/ircCommands.jsm
@@ +284,5 @@
> name: "mode",
> get helpString() _("command.modeUser", "mode") + "\n" +
> _("command.modeChannel", "mode"),
> run: function(aMsg, aConv) {
> + function isMode(aString) "+-".includes(aString[0]);
Can I ask you to change this to
let isMode = aString => "+-".includes(aString[0]);
while you are at it, or we'll just get a different warning? Looks like this line was overlooked in the recent removal of this kind of deprecated short function definiton.
Assignee | ||
Comment 14•10 years ago
|
||
Attachment #8600286 -
Flags: review?(neil)
Comment 15•10 years ago
|
||
Comment on attachment 8600279 [details] [diff] [review]
chat, v1
Review of attachment 8600279 [details] [diff] [review]:
-----------------------------------------------------------------
You can ignore my previous comment, as it seems the removal of expression closures hasn't landed yet for /chat.
Attachment #8600279 -
Flags: review?(aleth) → review+
Updated•10 years ago
|
Attachment #8600278 -
Flags: review?(philipp) → review+
Comment 16•10 years ago
|
||
(In reply to Archaeopteryx [:aryx] from comment #11)
> Created attachment 8600283 [details] [diff] [review]
> suite/browser, v1
Isn't there some in suite/browser/navigator.js?
Flags: needinfo?(archaeopteryx)
Comment 17•10 years ago
|
||
(In reply to Archaeopteryx [:aryx] from comment #12)
> Created attachment 8600284 [details] [diff] [review]
> suite/common, v1
Isn't there some in:
suite/common/downloads/downloadmanager.js
suite/common/search/search.xml
suite/common/sidebar/sidebarOverlay.js
Attachment #8600280 -
Flags: review?(iann_bugzilla) → review+
Comment 18•10 years ago
|
||
Comment on attachment 8600286 [details] [diff] [review]
suite/mailnews, v1
Stealing, r=me
Attachment #8600286 -
Flags: review?(neil) → review+
Assignee | ||
Comment 19•10 years ago
|
||
(In reply to Ian Neal from comment #16)
> (In reply to Archaeopteryx [:aryx] from comment #11)
> > Created attachment 8600283 [details] [diff] [review]
> > suite/browser, v1
>
> Isn't there some in suite/browser/navigator.js?
None of them are strings:
> line 694 -- if (!next || !next.classList.contains("nav-bar-class"))
> line 697 -- if (!previous || !previous.classList.contains("nav-bar-class"))
classLists
> line 2132 -- if (!title || frameset.document.styleSheetSets.contains(title)) {
DOMStringList
(In reply to Ian Neal from comment #17)
> (In reply to Archaeopteryx [:aryx] from comment #12)
> > Created attachment 8600284 [details] [diff] [review]
> > suite/common, v1
>
> Isn't there some in:
> suite/common/downloads/downloadmanager.js
> line 680 -- if (types.contains("text/uri-list") ||
> line 681 -- types.contains("text/x-moz-url") ||
> line 682 -- types.contains("text/plain"))
DOMStringList
> suite/common/search/search.xml
DOMStringList and classLists
> suite/common/sidebar/sidebarOverlay.js
classLists
Flags: needinfo?(archaeopteryx)
Updated•10 years ago
|
Attachment #8600281 -
Flags: review?(mkmelin+mozilla) → review+
Updated•10 years ago
|
Attachment #8600282 -
Flags: review?(mkmelin+mozilla) → review+
Assignee | ||
Comment 20•10 years ago
|
||
calendar: https://hg.mozilla.org/comm-central/rev/1f6cefb73596
chat: https://hg.mozilla.org/comm-central/rev/037c8b33c0fa
editor: https://hg.mozilla.org/comm-central/rev/6edc706e70ca
mail: https://hg.mozilla.org/comm-central/rev/c61af7cb544a
mailnews: https://hg.mozilla.org/comm-central/rev/1cf79e3da57a
suite/mailnews: https://hg.mozilla.org/comm-central/rev/0dd5e1453b7f
Attachment #8600283 -
Flags: review?(neil) → review+
Attachment #8600284 -
Flags: review?(neil) → review+
Assignee | ||
Comment 22•10 years ago
|
||
suite/browser: https://hg.mozilla.org/comm-central/rev/5e4860468aba
suite/common: https://hg.mozilla.org/comm-central/rev/8f08e3298ab8
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-thunderbird39:
--- → unaffected
status-thunderbird40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 40.0
You need to log in
before you can comment on or make changes to this bug.
Description
•