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

Revert "adxcgBidAdapter: updated backend protocol for 5.2x-legacy" #7829

Merged
merged 1 commit into from
Dec 8, 2021

Conversation

ChrisHuie
Copy link
Collaborator

@ChrisHuie ChrisHuie commented Dec 8, 2021

This seems to be failing on the legacy branch. Reverting to review #7786. Adding Pat for visibility

…50 in 6.x) for 5.2x-legacy (#7786)"

This reverts commit 612625d.
@ChrisHuie ChrisHuie merged commit c00ab53 into 5.20.x-legacy Dec 8, 2021
@adxcgcom
Copy link
Contributor

adxcgcom commented Dec 9, 2021

Solved the problem. It was browser (IE11) specific and showed only in browserstack tests., but not in local (gulp test that uses local chrome).

The problem was in "gulp test --browserstack " running IE11 that request.site.domain is empty "" , while in "gulp test" it's "localhost". All other browserstack enabled browsers work without problem.

    it('should send info about the site', function () {
      config.setConfig({
        site: {
          id: '123123',
          publisher: {
            domain: 'publisher.domain.com'
          }
        },
        ortb2: {
          site: {
            publisher: {
              id: 4441,
              name: 'publishers name'
            }
          }
        }
      });
      let validBidRequests = [{
        bidId: 'bidId',
        params: {adzoneid: '1000'}
      }];
      let refererInfo = {referer: 'page'};
      let request = JSON.parse(spec.buildRequests(validBidRequests, {refererInfo}).data);

      let expected = {
        id: '123123',
        publisher: {
          domain: 'publisher.domain.com',
          id: 4441,
          name: 'publishers name'
        },
        page: 'page',
        domain: 'localhost'
      };

      assert.deepEqual(request.site.id, expected.id);
      assert.deepEqual(request.site.publisher, expected.publisher);
      assert.deepEqual(request.site.page, expected.page);
      // assert.deepEqual(request.site.domain, expected.site.domain); // disabled for browser specific problem (IE11)
      // in browserstack IE 11 we get empty "" request.site.domain, while in gulp test we get "localhost"
      // all other browserstack enabled browsers work without problem
    });

Should we leave comments for clarity or remove them in new PR?

@adxcgcom
Copy link
Contributor

adxcgcom commented Dec 9, 2021

#7844

karimMourra pushed a commit to jwplayer/Prebid.js that referenced this pull request Feb 7, 2022
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