Content-Length: 137941 | pFad | https://bugzilla.mozilla.org/1920145

0 1920145 - Add automatic fluent support for the heading and message properties of moz-message-bar
Closed Bug 1920145 Opened 4 months ago Closed 2 months ago

Add automatic fluent support for the heading and message properties of moz-message-bar

Categories

(Toolkit :: UI Widgets, enhancement)

enhancement

Tracking

()

RESOLVED FIXED
134 Branch
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.

Mentor: hjones

May I take this issue if its still available for assignment?

Flags: needinfo?(hjones)

Hi Spencer! Thanks for your interest - feel free to pick up the bug and let me know if you have any questions about it

Flags: needinfo?(hjones)

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?

Assignee: nobody → speneth1
Status: NEW → ASSIGNED

(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.

Flags: needinfo?(speneth1)

(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.

Flags: needinfo?(speneth1) → needinfo?(hjones)

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.

Pushed by hjones@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6657459275f2 [moz-message] Modified moz-message property definitions, fluent set to true. r=hjones,settings-reviewers,credential-management-reviewers,reusable-components-reviewers,translations-reviewers,backup-reviewers,mtigley,fxview-reviewers,sidebar-reviewers,mossop,mconley
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 134 Branch
Flags: needinfo?(hjones)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size:









ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://bugzilla.mozilla.org/1920145

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy