Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#427 closed defect (fixed)

Remove usage of non-standard function syntax

Reported by: tschuster Assignee: tschuster
Priority: P3 Milestone: Adblock-Plus-2.6.4-for-Firefox
Module: Core Keywords:
Cc: trev Blocked By: #656
Blocking: #312, #517 Platform:
Ready: yes Confidential: no
Tester: Verified working: no
Review URL(s):

http://codereview.adblockplus.org/6305806509146112
http://codereview.adblockplus.org/5417063522762752/

Description

Firefox has these three non-standard syntax extensions:
function f(x) x + 1;
{get() 1} {set(v) v = 1;}

Change History (12)

comment:1 Changed 6 years ago by tschuster

  • Review URL(s) modified (diff)

comment:2 Changed 6 years ago by philll

You might want to provide a bit more detail in your issues in future to enable testers to work with it.

comment:3 Changed 6 years ago by tschuster

Ok, it's not clear to me what one would manually? test here. This is purely a code change that shouldn't have any observable effects. Should I just say that?

comment:4 Changed 6 years ago by philll

Even if there is no direct change of functionality, it should be obvious to testers in general, which functional parts a code change belongs to such that they can verify them still working as before. That's basically, why there should be a background section in every change issue. Also, if not sufficiently obvious, a "what to change" section might make sense.

comment:5 Changed 6 years ago by trev

  • Blocking 517 added

comment:6 Changed 5 years ago by tschuster

  • Owner set to tschuster

comment:8 Changed 5 years ago by tschuster

  • Resolution set to fixed
  • Status changed from new to closed

comment:9 follow-up: Changed 5 years ago by barbaz

With this change, latest dev builds now require at least Gecko 22, see https://developer.mozilla.org/docs/Web/JavaScript/Reference/arrow_functions#Browser_compatibility

comment:11 in reply to: ↑ 9 Changed 5 years ago by trev

  • Component changed from Unknown to Core
  • Milestone set to Adblock-Plus-for-Firefox-next
  • Priority changed from Unknown to P3
  • Ready set

Replying to barbaz:

With this change, latest dev builds now require at least Gecko 22, see https://developer.mozilla.org/docs/Web/JavaScript/Reference/arrow_functions#Browser_compatibility

Big ouch :-(

Adblock Plus 2.6.2 was already out, had to respin the release because of this - changing compatibility with this release wasn't the idea.

https://hg.adblockplus.org/adblockplus/rev/a7ef39c39c3b - backed out this change
https://hg.adblockplus.org/adblockplus/rev/ce3d000d9178 - relanded after the release, this time with appropriate compatibility info changes.

comment:12 Changed 5 years ago by tschuster

  • Blocked By 656 added
Note: See TracTickets for help on using tickets.