Ensure that reset() can be called at any time #61
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The originalFavicon variable is populated by getCurrentFavicon which was called only when setting a bubble, not when setting a custom image or resetting without setting the bubble. This avoids setting the favicon href to an empty string in these 2 cases, thus trying to use the current page as favicon.
|
@tommoor is there anything missing for this to be merged ? |
|
@tommoor ping |
When the head contains many favicon tags (needed to support all variants of modile and Windows 8 icons), the update of the live node list might be fast enough to be done before the iteration is completed. This means that some icons are not removed, which can break the resetting depending on the browser. The list is now iterated backward so that updates don't change the indexes of remaining elements.
|
I fixed another issue which was breaking the reset in Chrome. The commit explains it. Btw, this live NodeList update was probably the reason why the undfined check was needed. |
|
@tommoor is there any interest in merging this ? |
|
Check out my PR #69 - I ran into the same issue as you, but I think mine might solve this without running as much code. |
|
@ChrisMBarr there is still an issue in your case, in case an image is set: the originalFavicon variable is not populated, so resetting would not work |
|
Tidying up issues here, thanks for the PR! I ended up merging #69 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The originalFavicon variable is populated by getCurrentFavicon which was called only when setting a bubble, not when setting a custom image or resetting without setting the bubble.
This avoids setting the favicon href to an empty string in these 2 cases, thus trying to use the current page as favicon.