Multi-account-containers: Tooltip shows HTML entities for some characters

Created on 27 Sep 2019  路  9Comments  路  Source: mozilla/multi-account-containers

  • Multi-Account Containers Version: 6.1.0
  • Operating System + Version: Windows 10 1809
  • Firefox Version: 69.0.1, 70.0b9 (dev edition)
  • Other installed Add-ons + Version + Enabled/Disabled-Status: none in dev edition

Actual behavior

The tooltip on containers in the container list displays HTML entities for some characters, for example, container a/b c has the tooltip a/b c

Screenshots:

image

image

Expected behavior

The tooltip should display the same text as in the container list

Steps to reproduce

  1. Create container with special characters like & or /
  2. Open Multi-Account-Container addon dropdown from Firefox toolbar
  3. Hover mouse on a container
outreachy-assigned

Most helpful comment

Hey @maxxcrawford @groovecoder I have submitted a PR for this issue.

All 9 comments

Hi, I'm an Outreachy applicant and I would like to work on this :)
@groovecoder @maxxcrawford could I be assigned this please?

@Jo-IE Assigned. Thanks for contributing!

Hey @maxxcrawford I'm an Outreachy applicant & looking forward to contributing to this project but there're very few bugs with outreachy tag which are already taken so can you assign any bug to me or point me to a direction for contribution.

Hi @maxxcrawford I have made the necessary edits in popup.js to resolve this issue. Currently, the title attribute of the tr element is being passed into an escaped function to replace HTML characters with the entities seen on hover. What I have done is set the tooltip attribute without using that escaped function. I am just making sure there isn't anything else I'm missing before I submit my PR. Thanks in advance!

I'm also getting this error when I run npm test Do you have any pointers for me? @maxxcrawford

I've tried deleting my node modules folder and package-lock.json file and running npm install but still no luck.

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] lint:js: `eslint .`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the [email protected] lint:js script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

@Jo-IE Go ahead and file the PR.

Not sure about the npm test results. Try running the same command on the master branch and see if you get the same error. I confirmed on my end and that test ran back correctly.

@bini11 We're only adding the outreachy-assigned tags to issues that are assigned, so any issue without that tag is available. Look through the open issues without that tag, and comment on one you'd like to contribute to!

@maxxcrawford so to resolve the error I deleted my local copy and fork of the repo and made a new fork and ran npm test before making any changes and the tests worked perfectly. However, I switched to a new branch and made the changes to resolve this PR and after making the changes I ran npm test on the new branch and I'm getting those errors. I finally had to manually do node ./node_modules/eslint/bin/eslint --fix ./src/js/popup.js and after that npm test worked again

Hey @maxxcrawford @groovecoder I have submitted a PR for this issue.

Was this page helpful?
0 / 5 - 0 ratings