-
Notifications
You must be signed in to change notification settings - Fork 3
Implement viewport meta parsing with Device class integration and initial-scale support #237
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
Conversation
Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
…n and integration tests Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
src/client/browser/window.cpp
Outdated
| if (viewport_meta.device_width) | ||
| { | ||
| // Use device width - keep current values for simplicity | ||
| // In a real implementation, this would get actual device dimensions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot This could be implemented at client_cssom::Device(), you can add static fields DeviceWidth, DeviceHeight and DeviceDepth to represent the device size for such case: width=device-width.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented as requested. Added static DeviceWidth, DeviceHeight, and DeviceDepth fields to the client_cssom::Device class, and updated the device-width logic to use Device::DeviceWidth instead of placeholder comments. Commit f6b9e29.
src/client/browser/window.cpp
Outdated
| if (viewport_meta.device_height) | ||
| { | ||
| // Use device height - keep current values for simplicity | ||
| // In a real implementation, this would get actual device dimensions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto, implement this at client_cssom::Device().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented device-height logic using the client_cssom::Device class with the new static DeviceHeight field. The logic now properly uses device dimensions instead of placeholder comments. Commit f6b9e29.
src/client/browser/window.cpp
Outdated
| } | ||
| } | ||
|
|
||
| // TODO: Apply scaling factors (initial-scale, minimum-scale, maximum-scale, user-scalable) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Please move the initial-scale, minimal-scale ... to the client_cssom::Device(), and implement the initial-scale at this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved all scaling factors (initial-scale, minimum-scale, maximum-scale, user-scalable) to the client_cssom::Device class and implemented the initial-scale functionality. The initial-scale now properly affects the devicePixelRatio when set. Commit f6b9e29.
src/client/browser/window.cpp
Outdated
| // This would trigger a re-layout of the document | ||
| if (client_context_) | ||
| { | ||
| // TODO: Add RPC call to notify about viewport size change |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // TODO: Add RPC call to notify about viewport size change | |
| // TODO(yorkie): Add RPC call to notify about viewport size change |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the TODO comment format as suggested. Changed to use the TODO(yorkie): format for consistency. Commit f6b9e29.
- Add static DeviceWidth, DeviceHeight, DeviceDepth fields to Device class - Add scaling properties (initial-scale, minimum-scale, maximum-scale, user-scalable) to Device - Move device-width/device-height logic from Window to Device class - Implement initial-scale functionality that affects devicePixelRatio - Update setViewportWidth/Height methods to support device dimension flags - Update TODO comment as requested in review Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
This PR implements comprehensive viewport meta tag parsing in
HTMLMetaElementaccording to the MDN specification, with full integration into theclient_cssom::Deviceclass and initial-scale functionality.Key Features
Viewport Meta Parsing
width,height,initial-scale,minimum-scale,maximum-scale,user-scalabledevice-widthanddevice-heightDevice Class Integration
DeviceWidth,DeviceHeight,DeviceDepth) for proper device-width/device-height supportinitial-scalefunctionality that directly affectsdevicePixelRatiosetViewportWidth/setViewportHeightmethods with device dimension flag supportWindow Dimension Integration
widthandheightvalues using Device class methodsDynamic Updates
Document::onNodeAddedExample Usage
Implementation Details
parseViewportMeta()method and viewport change notificationsapplyViewportMeta()method to use Device class methods for all viewport operationsonViewportMetaChanged()handler and automatic detection inonNodeAddedThe implementation properly handles device dimensions using the actual device constants (1920x1080x400) and implements initial-scale functionality that affects the device pixel ratio for proper scaling behavior.
Testing
Comprehensive test coverage includes:
fixtures/viewport-meta-test.html) for visual verificationBrowser Compatibility
Follows MDN specification for viewport meta parsing with full backwards compatibility. The implementation handles edge cases gracefully and maintains existing window dimension behavior when no viewport meta is present.
Fixes #236.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.