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

isSafariBrowser update #5077

Merged
merged 4 commits into from
Apr 8, 2020
Merged

isSafariBrowser update #5077

merged 4 commits into from
Apr 8, 2020

Conversation

bretg
Copy link
Collaborator

@bretg bretg commented Apr 5, 2020

Type of change

  • Bugfix

Description of change

Per issue #5033, the isSafariBrowser function didn't handle the case of Chrome or Firefox on iOS

Other information

There weren't any unit tests for utils.isSafariBrowser() -- added

@bretg bretg requested a review from idettman April 5, 2020 22:13
@bretg
Copy link
Collaborator Author

bretg commented Apr 5, 2020

@idettman - I tried to save the navigator object and restore it, but received this error:

  Uncaught TypeError: Cannot assign to read only property 'navigator' of object '#<Window>'
  at webpack:///test/spec/utils_spec.js:948:14 <- test/test_index.js:332851:15

Copy link
Contributor

@r-ishigami r-ishigami left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@idettman
Copy link
Contributor

idettman commented Apr 6, 2020

@bretg I made a change to use sinon for stubbing the navigator.userAgent

Copy link
Contributor

@idettman idettman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@idettman idettman added LGTM and removed needs review labels Apr 6, 2020
@idettman idettman added needs 2nd review Core module updates require two approvals from the core team bug and removed bug labels Apr 7, 2020
@bretg bretg requested a review from Fawke April 7, 2020 14:40
@bretg bretg assigned Fawke and unassigned idettman Apr 7, 2020
Copy link
Contributor

@Fawke Fawke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Fawke Fawke removed the needs 2nd review Core module updates require two approvals from the core team label Apr 8, 2020
@Fawke Fawke merged commit c61a2b9 into master Apr 8, 2020
rjvelicaria pushed a commit to openx/Prebid.js that referenced this pull request Apr 9, 2020
* isSafariBrowser-update

* can't restore navigator object

* update tests to reset navigator.userAgent value to initial value after each test that modifies the value

* fix stubs the navigator userAgent property using sinon

Co-authored-by: idettman <[email protected]>
redaguermas added a commit to redaguermas/Prebid.js that referenced this pull request Apr 16, 2020
* 'master' of https://github.com/prebid/Prebid.js: (102 commits)
  Marsmedia - Add vastXml and fix id response (prebid#5067)
  PubMatic adapter to support image sync (prebid#5104)
  minor consentManagement fix (prebid#5050)
  fix circle ci failing tests (prebid#5113)
  Add Relaido Adapter (prebid#5101)
  Add new bid adapter for ConnectAd (prebid#4806)
  change payload (prebid#5105)
  Utils updates (prebid#5092)
  Read OpenRTB app objects if set in config + bug fix for when ad units are reloaded (prebid#5086)
  Criteo : added first party data mapping to bidder request (prebid#4954)
  updateAdGenerationManual (prebid#5032)
  New bid adapter: Wipes (prebid#5051)
  Prebid manager analytics utm tags (prebid#4998)
  CRITEO RTUS Integration with Yieldmo Prebid (prebid#5075)
  isSafariBrowser update  (prebid#5077)
  Support min &max duration for onevideo (prebid#5079)
  increment pre version
  Prebid 3.15.0 release
  prebid#5011 Fix to set Secure attribute on cookie when SameSite=none (prebid#5064)
  Prebid adapter for windtalker (prebid#5040)
  ...
iggyfisk pushed a commit to happypancake/Prebid.js that referenced this pull request Jun 22, 2020
* isSafariBrowser-update

* can't restore navigator object

* update tests to reset navigator.userAgent value to initial value after each test that modifies the value

* fix stubs the navigator userAgent property using sinon

Co-authored-by: idettman <[email protected]>
@idettman idettman deleted the isSafariBrowser-update branch March 8, 2021 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants