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.

Sign up for GitHub

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

Jump to bottom

firefox: Support paths for userChrome & userContent #3856

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

firefox: Support paths for userChrome & userContent #3856

wants to merge 1 commit into from
+12 −4

Conversation

amarshall
Copy link
Contributor

Description

A path may be preferred for some uses, and allowing it avoids the user needing to builtins.readFile, thus creating duplicates and making it more difficult to determine the actual store path.

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all.

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}

{long description}

    See CONTRIBUTING for more information and recent commit messages for examples.

  • If this PR adds a new module

    • Added myself as module maintainer. See example.

    • Added myself and the module files to .github/CODEOWNERS.

@amarshall amarshall requested a review from rycee as a code owner April 8, 2023 17:02
@stale
Copy link

stale bot commented Jul 9, 2023

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

@stale stale bot added the status: stale label Jul 9, 2023
@amarshall amarshall force-pushed the firefox-css-files branch from 2a55ba6 to 5789660 Compare July 11, 2023 22:38
@stale stale bot removed the status: stale label Jul 11, 2023
@stale
Copy link

stale bot commented Oct 11, 2023

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

@stale stale bot added the status: stale label Oct 11, 2023
@stale stale bot removed the status: stale label Oct 11, 2023
@amarshall
Copy link
Contributor Author

cc maintainers @rycee @kira-bruneau

@stale Stale
Copy link

stale bot commented Apr 10, 2024

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

@stale stale bot added the status: stale label Apr 10, 2024
@stale stale bot removed the status: stale label Apr 10, 2024
@stale Stale
Copy link

stale bot commented Jul 9, 2024

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

@stale stale bot added the status: stale label Jul 9, 2024
@amarshall
Copy link
Contributor Author

This is not stale. I actively patch these changes into my own config and use them. I appeased the stalebot for a year—I will do so no longer. If the PR has genuine merge conflicts, it will get updated.

@stale stale bot removed the status: stale label Jul 15, 2024
kira-bruneau
kira-bruneau approved these changes Jul 26, 2024
View reviewed changes
Copy link
Contributor

@kira-bruneau kira-bruneau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I missed this before. I don't have commit access, but this change looks good to me!

@stale Stale
Copy link

stale bot commented Nov 14, 2024

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

@nyabinary
Copy link

Bump does this still work with current firefox?

@kira-bruneau
Copy link
Contributor

kira-bruneau commented Feb 5, 2025 •
edited
Loading

I'll test this again, but from just the diff I don't think there's anything wrong.

CI was only failing on darwin because of some upstream issues in nixpkgs.

It'd be nice to have this merged!

kira-bruneau
kira-bruneau approved these changes Feb 5, 2025
View reviewed changes
Copy link
Contributor

@kira-bruneau kira-bruneau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tested this commit ontop of bd65bc3 (the HEAD of release-24.11), and everything looks good to me!

@kira-bruneau
Copy link
Contributor

kira-bruneau commented Feb 5, 2025 •
edited
Loading

Oh wait actually, I think it only worked because I have this defined in my settings:

toolkit.legacyUserProfileCustomizations.stylesheets = true

#5670 addresses this though

@amarshall amarshall force-pushed the firefox-css-files branch from 64efbca to 7599272 Compare March 2, 2025 13:21
@amarshall
firefox: Support paths for userChrome & userContent
4c3dcc7 
A path may be preferred for some uses, and allowing it avoids the user
needing to `builtins.readFile`, thus creating duplicates and making it
more difficult to determine the actual store path.
@amarshall amarshall force-pushed the firefox-css-files branch from 7599272 to 4c3dcc7 Compare March 2, 2025 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kira-bruneau kira-bruneau kira-bruneau approved these changes

@rycee rycee Awaiting requested review from rycee

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@amarshall @nyabinary @kira-bruneau