Opened 20 months ago

Closed 19 months ago

Last modified 19 months ago

#6552 closed change (fixed)

Add support for the 'externally_connectable' property for Chrome builds

Reported by: BrentM Assignee: tlucas
Priority: P2 Milestone:
Module: Automation Keywords:
Cc: sebastian, tlucas Blocked By:
Blocking: #6567 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29743581/
https://codereview.adblockplus.org/29756621/

Description (last modified by tlucas)

Background

In an upcoming release, AdBlock on Chrome will start using the Chrome message passing API - chrome.runtime.onMessageExternal. In order to do so, the buildtools need to support rendering appropriate values for
externally_connectable into the generated manifest.json

Since this isnt' the first time (and probably won't be the last time) that some arbitrary values need to be added to the manifest.json, we want to support some generic rendering from a metadata.* to a manifest.json

What to change

Implement a machinery which takes arbitrary key/value pairs from a manifest-section of a metadata.*-file and implicitly renders them into the generated manifest.json.

Let the parsing follow these rules:

  • An option's key may be declared as a series of nested dictionary keys, seperated by '.'.
  • Declaring an option's value in a new line (even if only one is given) will define the option's value as a list.
  • When an option's value is defined as a list, no other nested objects may follow.
  • A list is expandable by the ConfigParser's '+=' token (Note: A previously declared string will be converted into a list).
  • Values may be marked as number or bool by prefixing them accordingly (this also applies to values in a list):
    • bool:<value>
    • number:<value>

Integration Notes

Values in metadata.* that are supposed to be lists must now separated by newlines, instead of spaces (with the first items starting in a newline after the option itself).

Attachments (2)

packagerChrome.py (14.2 KB) - added by BrentM 20 months ago.
manifest.json.tmpl (3.5 KB) - added by BrentM 20 months ago.

Download all attachments as: .zip

Change History (18)

Changed 20 months ago by BrentM

Changed 20 months ago by BrentM

comment:1 Changed 20 months ago by BrentM

I've attached files that have been updated with what I believe are the required changes.

comment:2 Changed 20 months ago by sebastian

  • Cc tlucas added
  • Priority changed from Unknown to P2
  • Ready set

comment:3 Changed 20 months ago by tlucas

  • Owner set to tlucas

Hey Brent, thanks for providing the changes!

I wonder if you intentionally only added the matches-key?
FWIW i'll add ids and accepts_tls_channel_id too, to fully support externally_connectable-

comment:4 Changed 20 months ago by tlucas

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:5 follow-up: Changed 20 months ago by BrentM

I only did the minimum amount that we needed :) Thanks for adding the other options.

comment:6 in reply to: ↑ 5 Changed 20 months ago by tlucas

  • Description modified (diff)

Replying to BrentM:

I only did the minimum amount that we needed :) Thanks for adding the other options.

Alright - i adjusted the description accordingly.

comment:7 Changed 20 months ago by tlucas

  • Blocking 6567 added

comment:8 Changed 20 months ago by tlucas

  • Description modified (diff)

comment:9 Changed 20 months ago by tlucas

  • Description modified (diff)

comment:10 Changed 20 months ago by tlucas

  • Description modified (diff)

comment:11 Changed 19 months ago by abpbot

A commit referencing this issue has landed:
Issue 6552 - Support arbitrary manifest values

comment:12 Changed 19 months ago by tlucas

  • Description modified (diff)
  • Resolution set to fixed
  • Status changed from reviewing to closed

@Brent, the patch is now in master.
Please have a look at the example metadata in tests, it should be exactly what you need:

https://hg.adblockplus.org/buildtools/diff/8055aac42982/tests/metadata.chrome#l1.26

Best, Tristan

comment:13 Changed 19 months ago by tlucas

  • Review URL(s) modified (diff)

comment:14 Changed 19 months ago by abpbot

A commit referencing this issue has landed:
Issue 6552 - call setdefault() on correct dictionary

comment:15 Changed 19 months ago by tlucas

  • Description modified (diff)

comment:16 Changed 19 months ago by BrentM

Thanks I'll take a look.

Note: See TracTickets for help on using tickets.