I just realized I've forgotten to attach the current version of the 
writing-mode property patch, so here :>

Attachment: 0001-Add-support-for-parsing-the-writing-mode-property-v2.patch
Description: Binary data

On 2013-06-11, at 9:54 AM, Caitlin Potter <[email protected]> wrote:

> 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.
>> 
>> 
>> 
>> 
> 

Reply via email to