Fix quick fixes / known bugs from TODO section 1 - #9
Open
aaralh wants to merge 12 commits into
Open
Conversation
The computation was misindented into the no-parent branch of computed_style, so any element with a parent got None back and the property was silently dropped by the cascade.
raise NotImplemented is a TypeError at runtime since NotImplemented is not an exception class.
bubbles and cancelable defaulted to the truthy tuple (False,) instead of False.
DomException subclassed BaseException and was never raised anywhere. It now subclasses Exception, carries its message through super().__init__, and Node.removeChild/appendChildBeforeElement raise it with name NotFoundError (per the DOM spec) instead of leaking a bare ValueError from the list operations.
convert_absolute_size_to_pixels was a stub that returned 16 for every keyword; small/large/etc. now map to the standard scale on a 16px medium base.
The width branches re-derived the font size from the raw style string and assigned a str in the rem/em arms, so int * str string-repeated into self.width and later int() calls raised ValueError. The height branches crashed on any non-px font-size value. Both now use self.font_size, which is already resolved to int pixels in __init__. rem still uses the element font size as its base like create_margin/ create_padding do; the proper root-based rem distinction is tracked in TODO section 3.
Every border branch in create_border wrote internal_padding, so the value ended up as whichever side was processed last — e.g. a lone border-top-width horizontally indented the first word of text. It is now derived once from the parsed left border, matching its sole use as TextLayout's x inset. The em branch of calclulate_border_width also now uses the resolved self.font_size instead of re-parsing the raw style string, which crashed on non-px font-size values. Left padding is intentionally still not included; tracked as follow-up.
The body was commented out so the method always returned (0, 0), leaving corner gaps whenever per-side borders had differing widths. Each border line is again extended by half the width of its perpendicular neighbours.
scroll_down's max-scroll and the scrollbar thumb updates in scroll_down, scroll_up and scrollbar_scroll all read get_window_size()[0] (width) where the viewport height belongs, letting the page scroll past its end on landscape windows.
raster() no longer dumps rules.txt and document.html into the working directory on every page load; drop leftover debug prints in ImageLayout and transform_color.
check_key wrapped load_webpage in a bare except: pass, so any network, parse or layout error on Enter produced no feedback at all. Catch Exception, log it, and keep the 'break' binding behavior. Also annotate the log helper so it passes strict mypy.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes all 11 items in TODO.md section 1 (quick fixes / known bugs), one commit per bug, with unit tests for everything testable headlessly.
Engine (
web/)computed_style, so the property was silently dropped for any element with a parent.raise NotImplemented→raise NotImplementedErrorintag_is_special(the constant is not an exception, so it raisedTypeError).EventInittrailing commas madebubbles/cancelabledefault to the truthy tuple(False,)instead ofFalse.DomExceptionnow subclassesException(wasBaseException), carries its message throughsuper().__init__, and is actually raised:Node.removeChild/appendChildBeforeElementraise it with nameNotFoundErrorper the DOM spec instead of leaking a bareValueError.Application (
browser/)strtofont_sizeand then multiplied it, string-repeating intoself.widthand crashing laterint()calls; the height branches crashed on any non-px font-size. Both now use the already-resolvedself.font_size. (rem still uses the element font size as its base, matchingcreate_margin/create_padding; the proper root-based distinction is tracked in TODO §3.)convert_absolute_size_to_pixelsreturned 16 for everything;xx-small…xxx-largenow map to the standard scale on a 16px medium base.scroll_down's max-scroll plus the scrollbar thumb updates inscroll_down/scroll_up/scrollbar_scrollall readget_window_size()[0], letting pages scroll past their end.internal_paddingclobbering — everycreate_borderbranch overwrote it (last write won), so e.g. a loneborder-top-widthhorizontally indented text. It's now derived once from the left border width, matching its sole use asTextLayout's x inset.DrawBorder.calculate_offsetbody was commented out (always(0, 0)); restored the corner-mitre logic so mixed-width borders meet at corners.raster()no longer writesrules.txt/document.htmlto the CWD on every load; removed stray prints inImageLayoutandtransform_color.check_key's bareexcept: passnow catchesExceptionand logs the failure.Tests & typing
test_computed_style,test_parser_utils,test_event,test_dom_exception,test_font_utils,test_layout_units); suite grows 47 → 76, all green via./test.sh.Follow-ups (out of scope here)
internal_paddingstill ignores left padding; padding handling is separate.rem(root font size) vsemdistinction — TODO §3.🤖 Generated with Claude Code