Skip to content

🏗 Update Local AMP extension to allow custom base URLs#24029

Merged
rsimha merged 2 commits intoampproject:masterfrom
westonruter:add/custom-url-chrome-extension
Aug 20, 2019
Merged

🏗 Update Local AMP extension to allow custom base URLs#24029
rsimha merged 2 commits intoampproject:masterfrom
westonruter:add/custom-url-chrome-extension

Conversation

@westonruter
Copy link
Copy Markdown
Member

This is split off from #23213.

When calling gulp serve you can specify a host to use that is different from the default localhost:8000 (default arguments --host=localhost and --port=8000). However, the Local AMP browser extension does not support anything but the default. This PR updates the extension popup to include a URL input field for the actual base URL to be supplied (e.g. https://amphtml.lndo.site/).

Importantly, this allows for the extension to be successfully used on HTTPS sites. Previously, I would get insecure mixed content warnings on HTTPS sites since the script URLs would be loaded from http://localhost:8000.

image

@westonruter
Copy link
Copy Markdown
Member Author

/cc @wassgha

@wassgha wassgha self-requested a review August 18, 2019 16:20
Copy link
Copy Markdown
Contributor

@wassgha wassgha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM based on review from the other PR

Copy link
Copy Markdown
Contributor

@rsimha rsimha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM based on @wassgha's review at #23213 (comment) :)

@rsimha rsimha merged commit e772ead into ampproject:master Aug 20, 2019
westonruter added a commit to westonruter/amphtml that referenced this pull request Aug 20, 2019
* 'master' of github.com:ampproject/amphtml: (32 commits)
  ✨ Make tweet id a bindable attribute (ampproject#23953)
  🏗 Update Local AMP extension to allow custom base URLs (ampproject#24029)
  🏗 Improve serving from non-localhost host (ampproject#24066)
  Preventing half pixels. (ampproject#24050)
  Update callout-vendors.js (ampproject#23218)
  🏗 Fixes to `check-package-manager.js` (ampproject#24060)
  Rename AMP_MODE to __AMP_MODE. (ampproject#24052)
  Story media performance metrics. (ampproject#23962)
  Updating Story amp-sidebar width documentation. (ampproject#23894)
  Fixes race condition in amp-video-iframe (ampproject#24033)
  Rename ampExtendedElements to __AMP_EXTENDED_ELEMENTS (ampproject#24056)
  🏗🚮 Enable property inlining (ampproject#24053)
  ✨amp-ads: Added optional params for Directadvert network (ampproject#23724)
  <amp-experiment> style mutation fix and improvment (ampproject#23669)
  🐛 Allow http protocol for noscript > img fallbacks for parity with amp-img (ampproject#21686)
  🏗 Refactor transform-log-asserts (ampproject#24028)
  Automatically preconnect to source origins on page loads. (ampproject#24045)
  Support visibility API in the ampdoc (ampproject#23799)
  Amphtml visual tests should use relative path against root (ampproject#24042)
  FIX: check all fields' dirtiness on AMP form init (ampproject#23978)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants