I still have not added any tests explicitly regarding the writing-mode property, however I have gotten everything to a stage where all tests are passing, which previously was not the case.
I could use some guidance writing parse/select tests for the property because even after studying the test programs and data files, it's not really obvious what the system is -- And beyond that, since I'm not really a CSS guru by any means, it is likely that I would write incorrect expectations when e.g. important is used. It seems that there will also be a few more properties to add, including 'text-orientation' and 'direction', I'm not sure if I should add those as part of this patch or perhaps add those in subsequent chunks. You're all busy people so when you have some time just give me a nod and I'll see what I can do. On 2013-06-07, at 5:12 AM, John-Mark Bell <[email protected]> wrote: > On Fri, 2013-06-07 at 04:09 -0400, Caitlin Potter wrote: >> This patch is intended to support the parsing and storage of the >> "writing-mode" property, which is one of the building blocks for >> vertical text in css (support for which is currently required by the >> HTML5 track element and obviously is a pretty important localization >> feature -- and just simply looks cool, too). > > Excellent. Thanks for your work on this. > > [...] > >> It is quite likely that there are things substantially wrong, >> including the lack of test data (I haven't figured out how to get the >> test suite to even build and run yet, `make test` seems to fail >> substantially here.) > > You're using a Mac, right? I've no current idea why make test isn't > working for you on that platform, as the test infrastructure is pretty > trivial. We're investigating that now. > >> So some review would be good, I'd like to get this approved by netsurf >> devs before trying to squeeze it into mozilla-servo. > > In general, this looks fine to me (modulo the lack of test data, which > you've mentioned above). There are a few minor whitespace nits, which > can be fixed when we merge the change in. Additionally, there's a stray > commented-out chunk in css__set_writing_mode_from_hint. This, too is > trivially fixed, so I'm not concerned by it. > > > J. > > > >
