Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Aug 20, 2019

Conversation

westonruter
Copy link
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
Member Author

/cc @wassgha

Copy link
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
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)
  ...
thekorn pushed a commit to edelight/amphtml that referenced this pull request Sep 11, 2019
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