search

interledger / web-monetization-extension

An open-source browser extension that enables Web Monetization.
Apache License 2.0
49 stars 3 forks source link

fix(content): dispatch MonetizationEvent instead of CustomEvent #346

Closed sidvishnoi closed 3 months ago

sidvishnoi commented 3 months ago

Context

Closes https://github.com/interledger/web-monetization-extension/issues/9

Changes proposed in this pull request

  1. Dispatch MonetizationEvent instead of CustomEvent with detail
  2. Keep supporting event.detail access for backward compat, but add a warning on accessing for first time.
  3. Rename internal event (for communication between content scripts) from monetization-v2 to __wm-ext-monetization, along with onmonetization-attribute-changed.
  4. Rename contentScript to polyfill
  5. Turn contentScript code from a string to regular TS file, so we can typecheck it like rest of code (~presently typechecking disabled on that file as out of scope of this PR~ added typechecking too)
  6. Now, MonetizationEvent interface is available on global.

A sample code:

<!doctype html>
<html lang="en">
  <head onmonetization="console.warn('head', event)">
    <meta charset="UTF-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <title>Web Monetization test</title>
    <link
      rel="monetization"
      href="https://ilp.rafiki.money/sidvishnoi-usd-2"
      onmonetization="console.log('inline', event)"
    />
  </head>
  <body >
    <h1>Web Monetization test</h1>
    <script>
      const link = document.querySelector('link[rel="monetization"]')
      // link.addEventListener('load', console.log)
      link.addEventListener('monetization', (event) => {
        console.log('1: event', event)
        console.log('2: event', event instanceof window.MonetizationEvent)
        console.log('3: event.detail', event.detail)
      })
      window.addEventListener('monetization', (ev) =>
        console.log('window:', ev)
      )
    </script>
  </body>
</html>
raducristianpopa commented 3 months ago

Extension builds preview

Name Link
Latest commit 7e3a01b0c0735dbc3abffdc5429c894a9411f540
Latest job logs Run #9562731289
BadgeDownload
BadgeDownload
sidvishnoi commented 3 months ago

Question: https://webmonetization.org/docs/references/monetizationevent/ says we emit amountSent with value and currency. There's no assetScale there. And how's amountSent different from receiveAmount?

raducristianpopa commented 3 months ago

Question: https://webmonetization.org/docs/references/monetizationevent/ says we emit amountSent with value and currency. There's no assetScale there. And how's amountSent different from receiveAmount?

The current implementation is not following the spec. Based on the spec (https://webmonetization.org/specification/#amountsent-attribute), the currency should only be an ISO4217 string. I forgot about this part when sending the details about a monetization event from the background to the content script. We should convert the amount value to a user readable format:

// We should be careful about floating point precision as well
const amountSent = receiveAmount.value * 10 ** -receiveAmount.assetScale

The monetization event should only contain the following properties:

receiveAmount should not be present.

sidvishnoi commented 3 months ago

the currency should only be an ISO4217 string

I think then the concern was crypto currencies not having those codes? Maybe something to figure out later?

raducristianpopa commented 3 months ago

the currency should only be an ISO4217 string

I think then the concern was crypto currencies not having those codes? Maybe something to figure out later?

This shouldn't really be an issue at the moment. Most ASE that enable WM for their user will probably run Rafiki, which has a constraint on the asset code as well - ISO4217: https://rafiki.dev/concepts/asset/.