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

Ad markup with <!DOCTYPE html> fails to render on mobile #134

Closed
rokostik opened this issue Mar 19, 2021 · 8 comments · Fixed by #172
Closed

Ad markup with <!DOCTYPE html> fails to render on mobile #134

rokostik opened this issue Mar 19, 2021 · 8 comments · Fixed by #172
Assignees
Labels
bug Something isn't working

Comments

@rokostik
Copy link

Describe the bug
If an ad markup contains the <!DOCTYPE html> preamble, the prescribe library (which postscribe uses to parse HTML: https://github.com/krux/prescribe) fails to parse the preamble and silently stops parsing and only the HTML up to that point is inserted into the document. This is a know and old issue with postscribe (krux/postscribe#358).

To Reproduce
From prebid server return any ad markup with <!DOCTYPE html> (eg. <!DOCTYPE html><p>test<p>) and inspect the DOM in chrome://inspect.
In my case I only see the comment and two additional divs that are made, but not the ad markup.
Removing the doctype preamble renders the ad as expected.

Expected behavior
Render the ad with doctype correctly as on the web with prebid.js.

Screenshots
Render if doctype included:
Screenshot 2021-03-19 at 11 21 34

Render without doctype:
Screenshot 2021-03-19 at 11 23 10

Smartphone:
Tested on the android emulator.

@bretg
Copy link
Contributor

bretg commented Apr 15, 2022

Can you recommend a solution to this issue @rokostik ? Do we just have to replace <!DOCTYPE html> with <html>?

@YuriyVelichkoPI - could you take a look?

@bretg bretg added the bug Something isn't working label Apr 15, 2022
@patmmccann
Copy link

patmmccann commented Apr 15, 2022

It seems we just strip that line using a regex before this line?

postscribe(document.body, markup, {

from the issue: html = html.replace(/<(?xml|(!DOCTYPE[^\>\[]+([[^\]]+)?))+[^>]+>/g, '');

@patmmccann
Copy link

It appears https://github.com/krux/postscribe/pull/507/files may also solve ; @nickjacob could you comment?

@nickjacob
Copy link

Hi @patmmccann -- that's true, our fork of prescribe skips over invalid/unrecognized HTML; this is the relevant commit -- AppMonet/prescribe@cfc81a3

We've used it a lot in mobile app environments, but it doesn't have great test coverage. I'm not currently using it (we just build an additional iframe & contentDocument.write when rendering asynchronously).

@nickjacob
Copy link

I can clean up my prescribe & postscribe forks and publish to NPM if you want to test the behavior on this

@nickjacob
Copy link

@patmmccann looks like you're right -- https://jsfiddle.net/76xcq12d/ the @appmonet/postscribe does handle the doctype in the ADM

I made a PR to prescribe with only the change that would fix this. Not sure if you want to go off of our fork? Looks like smaato also has a fork with some good changes. I just published @appmonet/postscribe to NPM, you could try it in universal creative like this:

"postscribe": "npm:@appmonet/postscribe@0.1.2"

So the import doesn't have to change

@dgirardi
Copy link
Collaborator

I am unable to reproduce, but I'm afraid I'm missing some context. I tried rendering the example markup (<!DOCTYPE html><p>test<p>), through "normal" prebid.js & PUC in an emulated android webview, and it renders correctly. What is the setup here - as far as PUC is concerned I believe it should be enough to set it up as per the prebid-mobile instructions, is there something else different about mobile?

@dgirardi
Copy link
Collaborator

Looking through the code, it appears that setting up ads for mobile is more complicated than I can afford, and not documented (at least not here), which explains why it was not seeing the issue. If I force use of postscribe rendering does fail, and #172 seems to fix it.

However this raises the question: why are we using postscribe? ads rendered "normally" seem to work fine in an android webview. It's also useless bytes for everyone not serving mobile ads. Is it a relic of the past?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
5 participants