Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

fix flash always-deny setting #8828

Merged
merged 1 commit into from
May 18, 2017
Merged

fix flash always-deny setting #8828

merged 1 commit into from
May 18, 2017

Conversation

diracdeltas
Copy link
Member

@diracdeltas diracdeltas commented May 10, 2017

fix #8826

Test Plan:

  1. enable flash
  2. go to http://homestarrunner.com/
  3. you should see a Flash notification bar. click 'remember this decision' and 'deny'
  4. close the tab and open http://homestarrunner.com/ in a new tab
  5. you should not see a Flash notification bar this time

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

Test Plan:

Reviewer Checklist:

Tests

  • Adequate test coverage exists to prevent regressions
  • Tests should be independent and work correctly when run indivually or as a suite ref

fix #8826

Test Plan:
1. enable flash
2. go to http://homestarrunner.com/
3. you should see a Flash notification bar. click 'remember this decision' and 'deny'
4. close the tab and open http://homestarrunner.com/ in a new tab
5. you should not see a Flash notification bar this time
@diracdeltas diracdeltas added this to the 0.15.400 milestone May 10, 2017
@diracdeltas diracdeltas self-assigned this May 10, 2017
@mrose17
Copy link
Member

mrose17 commented May 12, 2017

@diracdeltas & @bsclifton - i am pretty sure than whenever someone answers "No" to an "Allow to " and selects "Remember this decision", then it is never remembered regardless of . i don't think it is limited to flash, e.g., i have seen the same behavior for things like autoplay audio/video/etc.

@diracdeltas
Copy link
Member Author

autoplay has a different code path so it should be a separate issue

@mrose17
Copy link
Member

mrose17 commented May 12, 2017

grumble.

@darkdh
Copy link
Member

darkdh commented May 15, 2017

@mrose17 , you can try whole new autoplay for next version because #8848 just merged.

Allow -> Allow until tab closed
Allow and remember -> Allow until you delete the site setting in about:preferences#security
Deny -> keep blocking and won't bother you until refresh
Deny and remember -> keep blocking and won't bother you until you delete the site setting in about:preferences#security

@mrose17
Copy link
Member

mrose17 commented May 15, 2017

@DARdk - many thanks. i'll check the next RC!

Copy link
Contributor

@luixxiul luixxiul left a comment

Choose a reason for hiding this comment

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

Test plan works.

@diracdeltas
Copy link
Member Author

diracdeltas commented May 18, 2017

@mrose17 it appears you edited #8826 instead of opening a new issue. FWIW because this PR fixes the original issue (flash only), your issue will be closed automatically when it is merged. If you have a separate issue, please open a separate ticket with specific repro steps.

for code organization reasons, I don't want to fix the broader issue and the flash-only issue in the same PR.

@diracdeltas diracdeltas merged commit 8c73ba3 into master May 18, 2017
@diracdeltas diracdeltas deleted the fix/flash-always-deny branch May 18, 2017 21:35
@srirambv
Copy link
Collaborator

@diracdeltas Is this expected?
flash

Shouldn't it not allow to run flash after Deny is selected?

@srirambv
Copy link
Collaborator

Right click on the same tab where the notification was cancelled allows the flash component to run

@diracdeltas
Copy link
Member Author

Shouldn't it not allow to run flash after Deny is selected?

Nope, because 'Allow Once' in the screenshot overrides the first Deny setting.

@mrose17
Copy link
Member

mrose17 commented Jun 21, 2017

@srirambv & @diracdeltas - i can't parse

Shouldn't it not allow to run flash after Deny is selected?

because of the double-negative. can the question be stated more simply? thanks!

@diracdeltas
Copy link
Member Author

@mrose17 i think sriram was asking if the behavior in the screenshot is correct and i already answered

@mrose17
Copy link
Member

mrose17 commented Jun 21, 2017

@diracdeltas - thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow <site> to <action>?
5 participants