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

Cookie Prompt doesn't work #296

Closed
jbeich opened this issue Dec 2, 2017 · 7 comments
Closed

Cookie Prompt doesn't work #296

jbeich opened this issue Dec 2, 2017 · 7 comments

Comments

@jbeich
Copy link
Contributor

jbeich commented Dec 2, 2017

Cookie Prompt (#259) appears to only work from build directory, at least on GTK platforms.

cookie_prompt_broken

Steps to reproduce

  1. ./mach bootstrap
  2. ./mach build
  3. ./mach run "about:preferences#privacy"
  4. Set History -> Waterfox will: Use custom settings for history -> [*] Accept cookies from websites -> Keep until: ask me every time
  5. ./mach run https://en.wikipedia.org/
  6. "Confirm setting cookie" window appeared
  7. Click "Show Details"
  8. Everything looks OK
  9. Click "Allow"
  10. ./mach install
  11. waterfox "about:preferences#privacy"
  12. Repeat step 3
  13. waterfox https://en.wikipedia.org/
  14. Click "Show Details"
  15. Notice empty fields
  16. Click "Allow"
  17. Notice nothing happens (window didn't disappear)
  18. Clicking on other buttons (except "Hide Details") also does nothing
@criztovyl
Copy link

For tracking, this happens when I click on any button, from Browser Console (56.0.1):

TypeError: nsICookieAcceptDialog is undefined  cookieAcceptDialog.js:168:3
	cookieDeny chrome://cookie/content/cookieAcceptDialog.js:168:3
	doCancelButton chrome://global/content/dialogOverlay.js:38:11
	oncommand chrome://cookie/content/cookieAcceptDialog.xul:1:1

@MrAlex94
Copy link
Collaborator

MrAlex94 commented Mar 9, 2018

Yes, seems even though it's defined and the appropriate component is getting called, it doesn't see the definition! Can any of you see what may have gone wrong in the commit?

@juneyourtech
Copy link

juneyourtech commented Mar 25, 2018

I'd like to have some confirmation, if cookie prompts are going to be kept long-term, because I just noticed a revert of Savarese's code, with the explanation in code comments, that [cookie prompting] was 'not supported' (by whom?).

Update: @MrAlex94 , It seems from a subreddit three days ago, that you haven't abandoned it, but just disabled it, until the issue becomes bug-free / gets resolved for a working reimplementation.

I've just let Savarese know about this bug, so I'm hoping for a response.

@juneyourtech
Copy link

juneyourtech commented Mar 27, 2018

Savarese's response wrt the patch:

It looks like the wrong patch was applied. Or rather, the patch for Firefox 56.0.1 was applied to a source tree based on Firefox 57.0.x. The patch for Firefox 57.0.4 should have been used. Perhaps I had
not yet published a patch for 57.0.4 and someone used the latest one available at the time.

The problem, of course, is that the patch usually stops working with each major Firefox release and must be updated. I haven't upgraded to Firefox 59.0.x yet, but will release an updated patch for it when I do. I cannot spend additional time porting the patch to Waterfox, but anyone who wishes to is free to do so.

Savarese's latest cookie prompt restore patch is currently based on the Firefox 57.0 source tree.

cc: @criztovyl @MrAlex94

@grahamperrin
Copy link

@grahamperrin
Copy link

#538

@juneyourtech
Copy link

juneyourtech commented Sep 15, 2018

My best guess could be, that there was a version/code conflict with merging one of D.F. Savarese's patches. The windows without window titles and other appropriate information seem to suggest, that Waterfox's modal dialog implementation was different (older?) from that of whichever (newer?) version the merged patch supported.

@criztovyl , do you remember, which of Savarese's patches did you merge, and which upstream version did it correspond to? A possible solution is to test cookie prompting with an older patch, and then, if it works reliably in alpha/beta, release.

It should simply be possible to merge, build, and test all of Savarese's patches (here) until there's a patch that works. Merging from newest to oldest and testing should help to find out the right one.

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

No branches or pull requests

5 participants