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

Use browser's path for br-resume coookie #359

Merged
merged 6 commits into from
Jul 9, 2020

Conversation

mc2
Copy link
Contributor

@mc2 mc2 commented Jun 17, 2020

For multiple files in items, cookie paths written by code get broken and don't match with browser.
Use what the browser sees as the path, trimming any /page/... or /mode/...
Note: Required for multiple files in items petabox code

https://webarchive.jira.com/browse/WEBDEV-3542

Solution Summary:

  • Implement regex trimming /path/ or /mode/ from location.pathname to use for cookie path
  • Wrap in method for testability
  • Refactor legacy test using .call() and breaking this
  • Add unit tests of new wrapped regex

Testing
Should set/get cookies perfectly, independent of new petabox code to handle multiple files in items

@codecov
Copy link

codecov bot commented Jun 17, 2020

Codecov Report

Merging #359 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #359      +/-   ##
==========================================
+ Coverage   60.61%   60.64%   +0.02%     
==========================================
  Files          32       32              
  Lines        3428     3430       +2     
  Branches      649      649              
==========================================
+ Hits         2078     2080       +2     
  Misses       1084     1084              
  Partials      266      266              
Impacted Files Coverage Δ
src/js/plugins/plugin.resume.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 03d6528...3394c92. Read the comment docs.

@mc2 mc2 force-pushed the webdev-3542-files-in-item-resume-cookie branch from b2ac4d2 to 26e06c8 Compare June 27, 2020 22:13
Copy link
Contributor

@iisa iisa left a comment

Choose a reason for hiding this comment

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

Code looks good!
Can merge when @mc2 confirms QA has passed.

src/js/plugins/plugin.resume.js Show resolved Hide resolved
tests/plugins/plugin.resume.test.js Show resolved Hide resolved
@mc2 mc2 merged commit bee8169 into master Jul 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants