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.

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

chore: Update servo to c070372 #269

Merged
merged 6 commits into from
Jan 21, 2025

Conversation

dklassic
Copy link
Collaborator

@dklassic dklassic commented Jan 20, 2025

Accompanying the change of servo preferences, several changes has been made:

  • Removed resources/prefs.json as we don't have difference from default servo at the moment
  • Removed the support for devtools related settings using arguments
  • Multiple debug options are now part of preference

Copy link
Collaborator

@pewsheen pewsheen left a comment

Choose a reason for hiding this comment

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

You'll need to set servo config prefs when new Verso, it requires remapping CSS prefs now.

servo_config::prefs::set(preferences);

I think we can just new a default Preference and enable flexbox then we can remove prefs.json as well.

@dklassic dklassic marked this pull request as ready for review January 20, 2025 15:01
@dklassic dklassic requested a review from wusyong as a code owner January 20, 2025 15:01
src/verso.rs Outdated Show resolved Hide resolved
src/config.rs Show resolved Hide resolved
@pewsheen pewsheen added this pull request to the merge queue Jan 21, 2025
Merged via the queue into versotile-org:main with commit 88d3810 Jan 21, 2025
6 checks passed
@@ -32,8 +35,6 @@ pub struct CliArgs {
pub no_panel: bool,
/// Window settings for the initial winit window
pub window_attributes: WindowAttributes,
/// Port number to start a server to listen to remote Firefox devtools connections. 0 for random port.
pub devtools_port: Option<u16>,
Copy link
Contributor

@Legend-Master Legend-Master Jan 23, 2025

Choose a reason for hiding this comment

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

Why is this removal? Servo changed devtools_port from Opts to Preferences, we should set it on Preferences instead of removing this

Copy link
Collaborator Author

@dklassic dklassic Jan 23, 2025

Choose a reason for hiding this comment

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

With corresponding settings moved to Preference means we have to consciously have a new component maintaining preference if we want to keep these.

We can add them back later.

Copy link
Contributor

Choose a reason for hiding this comment

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

With corresponding settings moved to Preference means we have to consciously have a new component maintaining preference if we want to keep these.

I'm not sure what do you mean, can't we just change

servo_config::prefs::set(Preferences::default());

to

servo_config::prefs::set(Preferences {
    devtools_server_port: ...,
    devtools_server_enabled: ...,
    ..Default::default()
});

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, we can just add it back like that for now. It is just removed for the sake of updating servo and the fact that we currently don't have preference differences from servo.

@dklassic dklassic deleted the update-servo-c070372 branch January 23, 2025 04:20
@dklassic dklassic changed the title Update servo c070372 chore: Update servo to c070372 Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants