Skip to content

Ad type adverticum 3p script https/http support.#7176

Merged
lannka merged 2 commits intoampproject:masterfrom
Adverticum:http/https-fix
Jan 31, 2017
Merged

Ad type adverticum 3p script https/http support.#7176
lannka merged 2 commits intoampproject:masterfrom
Adverticum:http/https-fix

Conversation

@Adverticum
Copy link
Copy Markdown
Contributor

Our invocation script should be called without protocoll to support sites served through both http and https.

@jridgewell
Copy link
Copy Markdown
Contributor

/to @lannka

@lannka lannka self-assigned this Jan 25, 2017
@lannka
Copy link
Copy Markdown
Contributor

lannka commented Jan 25, 2017

@Adverticum i cannot get the example display an ad. it's blank. can you try to make it work locally? check this for how-to.

@Adverticum
Copy link
Copy Markdown
Contributor Author

@lannka i have a working axample on localhost. See attached screenshot below.
That banner I'm using for example has a bit delay at the start. Give it a few seconds.
screenshot from 2017-01-25 08-59-31

@Adverticum
Copy link
Copy Markdown
Contributor Author

The main question is that, if I'm includeing our invocation scrit withe the writeScript method and the given url has no protocol will it work on both http and https?

@Adverticum
Copy link
Copy Markdown
Contributor Author

@lannka could you check the example again? Are you seeing a blank white square or the "No ad" message from AMP?

@lannka
Copy link
Copy Markdown
Contributor

lannka commented Jan 26, 2017

This is what I got. Did you turn on any user targeting on that slot?

screenshot from 2017-01-26 10 38 18

The main question is that, if I'm includeing our invocation scrit withe the writeScript method and the given url has no protocol will it work on both http and https?

Yep, it should work for both.

@Adverticum
Copy link
Copy Markdown
Contributor Author

I just changed the example to a simple image banner. Hope you can see it now.
Also @lannka can you tell me the earliest time this fix can go live? Our clients need it.

@lannka lannka merged commit 25413ae into ampproject:master Jan 31, 2017
@lannka
Copy link
Copy Markdown
Contributor

lannka commented Jan 31, 2017

@Adverticum the change will be rolled out to canary this Tursday.

torch2424 pushed a commit to torch2424/amphtml that referenced this pull request Feb 14, 2017
* Ad type adverticum 3p script https/http support.

* example change
mrjoro pushed a commit to mrjoro/amphtml that referenced this pull request Apr 28, 2017
* Ad type adverticum 3p script https/http support.

* example change
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.

3 participants