Skip to content

print the backup codes tab and not the current one #2704

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.

Sign up for GitHub

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

Jump to bottom
Merged
rullzer merged 2 commits into from
Feb 15, 2017
Merged

print the backup codes tab and not the current one #2704

rullzer merged 2 commits into from
Feb 15, 2017

Conversation

@ChristophWurst
Copy link
Member

@ChristophWurst ChristophWurst commented Dec 15, 2016

Fixes #2662

Before, the current tab was printed (and closed), not the temporary one with the backup codes.

Should be backported IMO. cc @karlitschek

@mention-bot
Copy link

mention-bot commented Dec 15, 2016

@ChristophWurst, thanks for your PR! By analyzing the history of the files in this pull request, we identified @nickvergessen to be a potential reviewer.

@ChristophWurst ChristophWurst added this to the Nextcloud 12.0 milestone Dec 15, 2016
@karlitschek
Copy link
Member

karlitschek commented Dec 15, 2016

please backport 👍

@nickvergessen
Copy link
Member

nickvergessen commented Dec 15, 2016

Doesn't work here with FF:
mozilla.pdf

@ChristophWurst
Copy link
Member Author

ChristophWurst commented Dec 15, 2016

Doesn't work here with FF:
mozilla.pdf

Works here with 50.1.0 🤔

@ChristophWurst
Copy link
Member Author

ChristophWurst commented Dec 15, 2016 •
edited
Loading

Ouch. Doesn't work on Chromium either.

@ChristophWurst ChristophWurst added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Dec 15, 2016
@ChristophWurst ChristophWurst force-pushed the 2fa-fix-backup-codes-print branch from 6bc4b09 to f86c71b Compare December 19, 2016 12:48
@ChristophWurst
Copy link
Member Author

ChristophWurst commented Dec 19, 2016

For some reason Chrome doesn't allow the "old" approach anymore, it closes the tab immediately if a data url is used. Now I'm using the window object to write to its document. Apparently that works on all browsers I have tested:

  • FF 50
  • Chromium (default settings, e.g. pop-ups are blocked)
  • Gnome Web

I don't have IE nor Safari, we should test those too as I'm not too confident that it works everywhere. Maybe @MorrisJobke or @LukasReschke can help here 😁

@ChristophWurst ChristophWurst added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Dec 20, 2016
ChristophWurst
ChristophWurst commented Dec 20, 2016
View reviewed changes
apps/twofactor_backupcodes/js/settingsview.js
var data = this._getDownloadData();
var newTab = window.open('', t('twofactor_backupcodes', 'Nextcloud backup codes'));
newTab.document.write('<h1>' + t('twofactor_backupcodes', 'Nextcloud backup codes') + '</h1>');
newTab.document.write(data);
Copy link
Member Author

@ChristophWurst ChristophWurst Dec 20, 2016

Choose a reason for hiding this comment

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

cc @LukasReschke for possible security implications of writing to the document directly

@ChristophWurst
print and close the new tab, not the current one when printing backup…
e111015 
… codes

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@ChristophWurst
add title to backup codes print page
3295fe2 
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@ChristophWurst ChristophWurst force-pushed the 2fa-fix-backup-codes-print branch from 8fb43ce to 3295fe2 Compare January 17, 2017 09:32
@ChristophWurst
Copy link
Member Author

ChristophWurst commented Jan 17, 2017

rebased

@codecov-io
Copy link

codecov-io commented Jan 17, 2017

Current coverage is 53.92% (diff: 100%)

Merging #2704 into master will not change coverage

@@             master      #2704   diff @@
==========================================
  Files          1302       1302          
  Lines         80097      80097          
  Methods        7905       7905          
  Messages          0          0          
  Branches       1245       1245          
==========================================
  Hits          43189      43189          
  Misses        36908      36908          
  Partials          0          0

Powered by Codecov. Last update b1a8296...3295fe2

MorrisJobke
MorrisJobke approved these changes Jan 26, 2017
View reviewed changes
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Tested and works with Firefox, Safari and Chrome 👍

@MorrisJobke
Copy link
Member

MorrisJobke commented Feb 10, 2017

@nickvergessen @rullzer @schiessle @LukasReschke Mind to review this one?

@MorrisJobke
Copy link
Member

MorrisJobke commented Feb 15, 2017

@nickvergessen @rullzer @schiessle @LukasReschke Mind to review this one?

Ping

@rullzer rullzer merged commit 2d8b7b8 into master Feb 15, 2017
@rullzer rullzer deleted the 2fa-fix-backup-codes-print branch February 15, 2017 19:18
@MorrisJobke
Copy link
Member

MorrisJobke commented Feb 15, 2017

@ChristophWurst Please open a backport PR ;)

@ChristophWurst ChristophWurst mentioned this pull request Feb 16, 2017
Merged
@ChristophWurst
Copy link
Member Author

ChristophWurst commented Feb 16, 2017

@ChristophWurst Please open a backport PR ;)

Done: #3510 :-)

@MorrisJobke MorrisJobke mentioned this pull request Feb 17, 2017
Closed
@ChristophWurst ChristophWurst mentioned this pull request Feb 28, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rullzer rullzer rullzer approved these changes

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@nickvergessen nickvergessen Awaiting requested review from nickvergessen nickvergessen is a code owner

@LukasReschke LukasReschke Awaiting requested review from LukasReschke

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

@MariusBluem MariusBluem Awaiting requested review from MariusBluem

@irgendwie irgendwie Awaiting requested review from irgendwie

Assignees

No one assigned

Labels

3. to review Waiting for reviews bug

Projects

None yet

Milestone

Nextcloud 12.0

Development

Successfully merging this pull request may close these issues.

NC11.0.0: Printing backup codes fails when Adblock Plus is activated, ABP prevents the new window for printing the backup codes to be opened.

8 participants

@ChristophWurst @mention-bot @karlitschek @nickvergessen @codecov-io @MorrisJobke @rullzer