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

flash-msg.js may not work correctly when there is no flash message HTML #130

Closed
delphidabbler opened this issue Sep 28, 2022 · 2 comments
Closed
Assignees
Labels
bug Something isn't working completed Work has been completed and has been pushed to the develop branch

Comments

@delphidabbler
Copy link
Owner

delphidabbler commented Sep 28, 2022

Experimentation with cookies showed a cookie name of dd_flash_undefined was being tested for.

This implies that perhaps the code that detects for the absence of the flash-popup id is not working. Possibly the following test never evaluates true:

flash is set in the previous line:

var flash = $("#flash-popup");

If certain Jekyll variables are set then the flash HTML should not be present and so the flash-popup id should not be present.

The only way I can see that a dd_flash_undefined cookie name can be built is if flash.data('flash-id') returns undefined in the following code:

var id = flash.data('flash-id');
var cookieName = 'dd_flash_' + id;

Getting to this code should be impossible when there is no HTML.

So, this code needs to be checked.

@delphidabbler delphidabbler self-assigned this Sep 28, 2022
@delphidabbler delphidabbler added accepted Accepted this issue as a valid issue that will be actioned needs investigation Issue needs some investigation labels Sep 28, 2022
@delphidabbler
Copy link
Owner Author

I misunderstood how jQuery notifies a selector it doesn't find. I assumes $("#flash-popup") would return falsy if #flash-popup was not found.

What actually happens is that a valid object is returned with zero length.

So need to change the test:

if ( ! flash )

to

if ( flash.length === 0 )

@delphidabbler
Copy link
Owner Author

delphidabbler commented Sep 28, 2022

Will actually fix this bug while implementing fix required by issue #129

@delphidabbler delphidabbler added bug Something isn't working and removed needs investigation Issue needs some investigation labels Sep 28, 2022
@delphidabbler delphidabbler added in progress Work has started on this issue and removed accepted Accepted this issue as a valid issue that will be actioned labels Oct 8, 2022
@delphidabbler delphidabbler added completed Work has been completed and has been pushed to the develop branch and removed in progress Work has started on this issue labels May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working completed Work has been completed and has been pushed to the develop branch
Projects
None yet
Development

No branches or pull requests

1 participant