Add automatic fluent support for the heading and message properties of moz-message-bar
Categories
(Toolkit :: UI Widgets, enhancement)
Tracking
()
Tracking | Status | |
---|---|---|
firefox134 | --- | fixed |
People
(Reporter: hjones, Assigned: speneth1, Mentored)
References
(Blocks 1 open bug)
Details
(Keywords: good-first-bug, Whiteboard: [recomp][lang=js])
Attachments
(1 file)
moz-message-bar
supports heading
and message
reactive properties that are almost always set via fluent, which means that consumers need to specify data-l10n-attrs="heading, message"
like this:
<moz-message-bar data-l10n-id="my-id" data-l10n-attrs="heading, message">
Bug 1901336 made it possible to automatically wire up data-l10n-attrs
by specifying fluent: true
in the property definition. We should modify moz-message
to take advantage of this by changing these property definitions to
heading: { type: String, fluent: true },
message: { type: String, fluent: true }
There will then be some additional work to clean up data-l10n-attrs
on instances of moz-message-bar
in the codebase.
Updated•4 months ago
|
Updated•3 months ago
|
May I take this issue if its still available for assignment?
Reporter | ||
Comment 2•3 months ago
|
||
Hi Spencer! Thanks for your interest - feel free to pick up the bug and let me know if you have any questions about it
While I'm following the reviewer checklist, I'm noticing the recommendation of adding a unit test if possible. I personally don't have too much experience with Test Driven Development practices, so I'm not sure yet of how to recognize when its good to add a test or not for certain bug environment circumstances. Given this is my first bug and it seems to be just rewriting some definitions, maybe I'm overthinking the requirements.
Nonetheless, in trying to test out the XPCShell testing system by running it on an already existing test, I encountered a missing module error:
ModuleNotFoundError: No module named 'pipes'
File "/Users/speneth/Documents/MozFirefoxBuild/main/testing/xpcshell/mach_commands.py", line 283, in run_xpcshell_test
return xpcshell.run_test(**params)
~~~~~~~~~~~~~~~~~^^^^^^^^^^
File "/Users/speneth/Documents/MozFirefoxBuild/main/testing/xpcshell/mach_commands.py", line 46, in run_test
return self.run_suite(**kwargs)
~~~~~~~~~~~~~~^^^^^^^^^^
File "/Users/speneth/Documents/MozFirefoxBuild/main/testing/xpcshell/mach_commands.py", line 32, in run_suite
return self._run_xpcshell_harness(**kwargs)
~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^
File "/Users/speneth/Documents/MozFirefoxBuild/main/testing/xpcshell/mach_commands.py", line 50, in _run_xpcshell_harness
import runxpcshelltests
File "/Users/speneth/Documents/MozFirefoxBuild/main/testing/xpcshell/runxpcshelltests.py", line 10, in <module>
import pipes
Looking it up online it seems like the pipes
module is deprecated for Python 3.x, so when I try to install it into my virtual environment using pip, it isn't found. There are many references to it in various parts of my local repository however, which I'm not sure how to work around. This seems to be an issue with my local environment and so far it only specifically impedes the XPCShell testing process.
Should I even be worrying about this for a first bug though? I might be in over my head this early on. Otherwise, how can I resolve this?
Updated•3 months ago
|
Reporter | ||
Comment 5•2 months ago
|
||
(In reply to Spencer from comment #3)
While I'm following the reviewer checklist, I'm noticing the recommendation of adding a unit test if possible. I personally don't have too much experience with Test Driven Development practices, so I'm not sure yet of how to recognize when its good to add a test or not for certain bug environment circumstances. Given this is my first bug and it seems to be just rewriting some definitions, maybe I'm overthinking the requirements.
Nonetheless, in trying to test out the XPCShell testing system by running it on an already existing test, I encountered a missing module error:
ModuleNotFoundError: No module named 'pipes' File "/Users/speneth/Documents/MozFirefoxBuild/main/testing/xpcshell/mach_commands.py", line 283, in run_xpcshell_test return xpcshell.run_test(**params) ~~~~~~~~~~~~~~~~~^^^^^^^^^^ File "/Users/speneth/Documents/MozFirefoxBuild/main/testing/xpcshell/mach_commands.py", line 46, in run_test return self.run_suite(**kwargs) ~~~~~~~~~~~~~~^^^^^^^^^^ File "/Users/speneth/Documents/MozFirefoxBuild/main/testing/xpcshell/mach_commands.py", line 32, in run_suite return self._run_xpcshell_harness(**kwargs) ~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^ File "/Users/speneth/Documents/MozFirefoxBuild/main/testing/xpcshell/mach_commands.py", line 50, in _run_xpcshell_harness import runxpcshelltests File "/Users/speneth/Documents/MozFirefoxBuild/main/testing/xpcshell/runxpcshelltests.py", line 10, in <module> import pipes
Looking it up online it seems like the
pipes
module is deprecated for Python 3.x, so when I try to install it into my virtual environment using pip, it isn't found. There are many references to it in various parts of my local repository however, which I'm not sure how to work around. This seems to be an issue with my local environment and so far it only specifically impedes the XPCShell testing process.Should I even be worrying about this for a first bug though? I might be in over my head this early on. Otherwise, how can I resolve this?
Hey Spencer! Sorry I missed this comment - in the future it's helpful to use this form to request info if you have a question, since that is a better guarantee that someone will get notified.
In this case I don't think we need to add a test for this directly, since we would effectively be testing that the Fluent library is working as expected. We also already have tests for the base MozLitElement
class checking that Fluent attributes get added as expected, which effectively gives us coverage for this change.
For the error above, do you remember specifically which test or command you were running?
If you want to be sure your changes won't cause unexpected test failures, you can make a push to our try automation servers by running ./mach try
.
(In reply to Hanna Jones [:hjones] from comment #5)
(In reply to Spencer from comment #3)
While I'm following the reviewer checklist, I'm noticing the recommendation of adding a unit test if possible. I personally don't have too much experience with Test Driven Development practices, so I'm not sure yet of how to recognize when its good to add a test or not for certain bug environment circumstances. Given this is my first bug and it seems to be just rewriting some definitions, maybe I'm overthinking the requirements.
Nonetheless, in trying to test out the XPCShell testing system by running it on an already existing test, I encountered a missing module error:
ModuleNotFoundError: No module named 'pipes' File "/Users/speneth/Documents/MozFirefoxBuild/main/testing/xpcshell/mach_commands.py", line 283, in run_xpcshell_test return xpcshell.run_test(**params) ~~~~~~~~~~~~~~~~~^^^^^^^^^^ File "/Users/speneth/Documents/MozFirefoxBuild/main/testing/xpcshell/mach_commands.py", line 46, in run_test return self.run_suite(**kwargs) ~~~~~~~~~~~~~~^^^^^^^^^^ File "/Users/speneth/Documents/MozFirefoxBuild/main/testing/xpcshell/mach_commands.py", line 32, in run_suite return self._run_xpcshell_harness(**kwargs) ~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^ File "/Users/speneth/Documents/MozFirefoxBuild/main/testing/xpcshell/mach_commands.py", line 50, in _run_xpcshell_harness import runxpcshelltests File "/Users/speneth/Documents/MozFirefoxBuild/main/testing/xpcshell/runxpcshelltests.py", line 10, in <module> import pipes
Looking it up online it seems like the
pipes
module is deprecated for Python 3.x, so when I try to install it into my virtual environment using pip, it isn't found. There are many references to it in various parts of my local repository however, which I'm not sure how to work around. This seems to be an issue with my local environment and so far it only specifically impedes the XPCShell testing process.Should I even be worrying about this for a first bug though? I might be in over my head this early on. Otherwise, how can I resolve this?
Hey Spencer! Sorry I missed this comment - in the future it's helpful to use this form to request info if you have a question, since that is a better guarantee that someone will get notified.
In this case I don't think we need to add a test for this directly, since we would effectively be testing that the Fluent library is working as expected. We also already have tests for the base
MozLitElement
class checking that Fluent attributes get added as expected, which effectively gives us coverage for this change.For the error above, do you remember specifically which test or command you were running?
If you want to be sure your changes won't cause unexpected test failures, you can make a push to our try automation servers by running
./mach try
.
Hey Hanna! Thanks for replying back, apologies for not properly pinging my concerns from earlier! I will request info from now on if I have any other concerns, thanks for letting me know.
I've noted your concerns you listed in the revision request for my submitted commit, and I've further cleaned up references to data-l10n-attrs declarations involving "heading" and message. I've also changed the documentation that references heading and message per your suggestion.
I understand the lack of necessity for tests for this kind of bug fix, thank you for the clarification. Following your advice so I can continue to get an understanding of how the testing system works for Firefox, I pushed it to the try servers and had no issues running it through ./mach try
, thanks.
To test XPCShell locally, I basically just picked a test folder that was relatively close to my file path at the time, and ran this command:
./mach xpcshell-test ./toolkit/content/tests/unit
Admittedly though, I tried it on other test folders as well and kept getting the same error, so I figured it had to do with my python environment as it was referencing my venv's installed modules.
My issue here may ultimately be unrelated to this bug specifically, but I really appreciate your concern with this problem. If you have further help or advice for this, please let me know!
Also, pardon me but I was curious, I noticed when amending my commit and submitting again via moz-phab
, it creates a new differential that doesn't seem to reference the previous differential with your revision request details. Is this normal for the code review process? I'm hoping I didn't miss a step in linking all the relevant details together for this bug.
Also, pardon me but I was curious, I noticed when amending my commit and submitting again via
moz-phab
, it creates a new differential that doesn't seem to reference the previous differential with your revision request details. Is this normal for the code review process? I'm hoping I didn't miss a step in linking all the relevant details together for this bug.
Disregard that last part, I misunderstood the structure of the differential page, I understand it now.
Reporter | ||
Updated•2 months ago
|
Description
•