Remove `parseUrlDeprecated` from extensions

[alanorozco]

Sneaking in via src/iframe-helper.js and src/3p-frame.js.

Extensions affected: 3p (ads, twitter, facebook, etc.)

Initial attempt to fix: #16220

Problems possibly with trying to fetch service from a detached node:

Error: [amp-a4a] Error: amp-a4a: adsense: The node must be attached to request ampdoc

/cc @jridgewell @aghassemi

[jridgewell]

Need a stack trace.

[alanorozco]

https://travis-ci.org/ampproject/amphtml/jobs/395181672

          at console.<anonymous> (/tmp/test/_init_tests.js:327:12 <- /tmp/2c0a11e3f0cce62076f6d9b036ff8ee7.browserify:90349:13)
          at Object.invoke (node_modules/sinon/pkg/sinon.js:406:32)
          at console.functionStub (node_modules/sinon/pkg/sinon.js:2908:47)
          at Function.invoke (node_modules/sinon/pkg/sinon.js:2504:51)
          at console.proxy [as error] (node_modules/sinon/pkg/sinon.js:2390:22)
          at Log.msg_ (/tmp/src/log.js:164:9 <- /tmp/2c0a11e3f0cce62076f6d9b036ff8ee7.browserify:59557:10)
          at Log.error_ (/tmp/src/log.js:219:11 <- /tmp/2c0a11e3f0cce62076f6d9b036ff8ee7.browserify:59622:12)
          at Log.error (/tmp/src/log.js:234:30 <- /tmp/2c0a11e3f0cce62076f6d9b036ff8ee7.browserify:59638:29)
          at MockA4AImpl.promiseErrorHandler_ (/tmp/extensions/amp-a4a/0.1/amp-a4a.js:1042:13 <- /tmp/2c0a11e3f0cce62076f6d9b036ff8ee7.browserify:6365:24)
          at /tmp/extensions/amp-a4a/0.1/amp-a4a.js:1114:11 <- /tmp/2c0a11e3f0cce62076f6d9b036ff8ee7.browserify:6441:14

[jridgewell]

Likley because they're not attaching the node to a doc, or that doc doesn't have an associated ampdoc.

[alanorozco]

Yep.

[erwinmombay]

This issue doesn't have a category which makes it harder for us to keep track of it. @alanorozco Please add an appropriate category.

[rsimha]

Seems like this is a runtime issue? (Please reassign if I'm wrong.)

[erwinmombay]

For the esm build we can just use the URL api

[jridgewell]

For the esm build we can just use the URL api

We can look at making url.js use the URL api directly, but we still need the url service to handle differences between main doc, FIE, and shadow roots. Essentially, we'd only be saving https://github.com/ampproject/amphtml/blob/master/src/url.js#L126-L170, and we can't even get rid of the <a> allocation since the polyfill can't differentiate between the calling contexts correctly.

[kevinkimball]

A few observations from working on this issue:

1. In some cases we are using parseUrlDeprecated to parse the location object. This is unnecessary, as location already exposes accessors for origin, pathname, hash, etc. This use can simply be removed.
2. The browser gives us a URL class which can handle this for us. This is only missing from IE11. We should polyfill and then use URL when appropriate (e.g. when we have a full URL string). This would also be easier to use because we don't need to look up the url service provided by AMP.
3. Need to check, but if using parseUrlDeprecated is not on the denylist for PR check, it needs to be added to prevent this from being inadvertently added to additional extensions.

[alanorozco]

2. The browser gives us a URL class which can handle this for us. This is only missing from IE11. We should polyfill and then use URL when appropriate (e.g. when we have a full URL string). This would also be easier to use because we don't need to look up the url service provided by AMP.

Handled on #31594

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.