Make RangeInputType into a three-part flexible box, replacing crufty layout code
Created attachment 253952 [details] Patch
Will fix up the efl-wk2 error in a subsequent patch.
Comment on attachment 253952 [details] Patch Attachment 253952 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/4720096303906816 New failing tests: fast/forms/box-shadow-override.html fast/forms/input-appearance-height.html fast/forms/range/range-change-min-max.html
Created attachment 253954 [details] Archive of layout-test-results from ews105 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Comment on attachment 253952 [details] Patch Attachment 253952 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5103627219763200 New failing tests: fast/forms/box-shadow-override.html fast/forms/input-appearance-height.html fast/forms/range/range-change-min-max.html
Created attachment 253957 [details] Archive of layout-test-results from ews100 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-mavericks Platform: Mac OS X 10.9.5
Created attachment 254003 [details] Patch
Added a couple of mac-mavericks rebaselines, and fixed an issue where the tracks children weren't being marked as needing layout.
Created attachment 254007 [details] Patch
Comment on attachment 254007 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=254007&action=review > Source/WebCore/html/HTMLInputElement.h:154 > HTMLElement* sliderThumbElement() const; > HTMLElement* sliderTrackElement() const; > + HTMLElement* sliderMinTrackElement() const; > + HTMLElement* sliderMaxTrackElement() const; Exposing all of these elements in public functions seems like a particularly bad way to create a relationship between type-specific renderer classes and HTMLInputElement. I see that you are participating in a grand tradition of questionable design here, but I still don’t like it. > Source/WebCore/rendering/RenderSlider.cpp:121 > + HTMLElement* sliderTrackElement = element().sliderTrackElement(); > + HTMLElement* minTrackElement = element().sliderMinTrackElement(); > + HTMLElement* maxTrackElement = element().sliderMaxTrackElement(); > + > + ASSERT(sliderTrackElement); > + ASSERT(minTrackElement); > + ASSERT(maxTrackElement); Can we get at these some other way than making them public functions on HTMLInputElement? Let me put this another way. This function clearly only needs to make style changes to these elements. So could we expose functions that return RenderStyle& instead of functions that return HTMLElement? Or we could add functions that expose the renderers. And could we make RenderSlider a friend of HTMLInputElement so these could be private functions? If we were going to keep the ones that return elements I would use a return type of HTMLElement& and have the assertion inside the function rather than at the call site.
(In reply to comment #10) > Comment on attachment 254007 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=254007&action=review > > > Source/WebCore/html/HTMLInputElement.h:154 > > HTMLElement* sliderThumbElement() const; > > HTMLElement* sliderTrackElement() const; > > + HTMLElement* sliderMinTrackElement() const; > > + HTMLElement* sliderMaxTrackElement() const; > > Exposing all of these elements in public functions seems like a particularly > bad way to create a relationship between type-specific renderer classes and > HTMLInputElement. I see that you are participating in a grand tradition of > questionable design here, but I still don’t like it. Yes, but I'm trying to make it less questionable, so this would be in the new tradition of less questionable design. :) Another option would be to put these methods on RangeInputType, and make RangeInput downcast-able from InputType. > > Source/WebCore/rendering/RenderSlider.cpp:121 > > + HTMLElement* sliderTrackElement = element().sliderTrackElement(); > > + HTMLElement* minTrackElement = element().sliderMinTrackElement(); > > + HTMLElement* maxTrackElement = element().sliderMaxTrackElement(); > > + > > + ASSERT(sliderTrackElement); > > + ASSERT(minTrackElement); > > + ASSERT(maxTrackElement); > > Can we get at these some other way than making them public functions on > HTMLInputElement? Yes, let me try the option I mention above. > Let me put this another way. This function clearly only > needs to make style changes to these elements. So could we expose functions > that return RenderStyle& instead of functions that return HTMLElement? Or we > could add functions that expose the renderers. And could we make > RenderSlider a friend of HTMLInputElement so these could be private > functions? I'll keep that in mind if I can't move these methods to RangeInputType. > If we were going to keep the ones that return elements I would use a return > type of HTMLElement& and have the assertion inside the function rather than > at the call site. I think this is a great idea regardless.
Created attachment 254022 [details] Patch for landing
Created attachment 254025 [details] Patch for landing