Don’t Complain, Fix It!

The worst thing you can do if you are using somebody else’s code is to complain to them via email about the shortcomings of their software. More often than not you can assume that the Open Source code this person put online is a labor of love. And complaining is as close as you can get to trampling on the other person’s feelings about their “baby”.

Hobbyists and Pros alike, you are a developer and if you have any interest in getting improvements for the other person’s software then you should adhere to these simple steps I am laying out for you to follow. This is specific to DTCoreText, but I am certain that the basic principles apply to any other project as well.

I won’t go into details about how pull requests are working on GitHub, there is an earlier extensive blog article on this subject at Cocoanetics.com. Rather I would like to walk you through the process I am going through fixing an issue on DTCoreText. The more of these steps you are willing to shoulder the happier the project’s maintainer will be.

Gather Data

When you encounter an issue you first should check if the problem is in your own code or really in the framework you are using. In DTCoreText there is a simple mechanism to check your specific problem. You put your HTML code into CurrentTest.html, which is the snippet linked to the last entry in the list of demo documents.

Launch the test app and open the bottom-most entry titled “Never mind this file …”. Never mind me stating that you shouldn’t touch this file. For helping to fix issue, I’ll make an exception. 😉

Turn on debug frame drawing via the button in the bottom left to see where the lines and glyph runs are.

The issue in this example relates to the paragraph style of these two paragraphs. Remember, each <p> tag turns into a line of text terminated with a \n, those are called paragraphs and they have a style that describes attributes that related to the entire paragraph, like for example writing direction.

You can see that the bug in this case is that both ways of representing the writing direction should be equivalent: specifying it via dir HTML attribute or via CSS style attribute.

The important thing about this step is that we a) are able to reliably reproduce the issue outside of the app we are currently working on and are able to demonstrate it in the demo app that comes with the component we are using. Never expect the developer to have the time or patience to deal with your personal code. But if you can provide test data you are a third of the way towards a good bug report.

Gather More Data

You don’t have to be satisfied with your research at the point of being able to reproduce the issue. If you reported it at this stage the developer would still have to carry out this second step. If you can take this one you save the other guy a great deal of work.

In this case we are talking about a paragraph attribute that does not get recognized, so we’ll dig further.

There are several views in the demo app for each document that help you gather more information:

• View – The basic rich text view
• Range – The Core Text attributes for each range
• Chars – The individual character codes
• HTML – How HTML looks generated from this attributed string via DTHTMLWriter
• iOS 6 – displays the attributed string in an iOS 6 UITextView with iOS6 tags enabled

Since both paragraphs in our sample have the same style (= attributes) for the entire line. So we switch to the Range view to see what paragraph attributes they got.

We can see that the NSParagraphStyle key has a CTParagraphStyle object attached and obviously the base writing direction value is different. -1 for the first paragraph and 1 for the second. In CTParagraphStyle.h we find:

typedef CF_ENUM(int8_t, CTWritingDirection) {
    kCTWritingDirectionNatural = -1,
    kCTWritingDirectionLeftToRight = 0,
    kCTWritingDirectionRightToLeft = 1
};

The first paragraph has retained the default “natural” writing direction, whereas the second paragraph got the correct Right-to-Left writing direction. This gives us enough information to properly formulate an …

Issue on GitHub

Bug reports should be formulated in a neutral tone. Refrain from calling the developer names or stating that his code is unfit for a certain purpose.

You can easily add proper formatting for statements that represent code by putting it between accent characters. A longer code block you can indent with 4 spaces to show up as formatted text. If you are not certain how it will look then switch to the Preview mode. Here the issue can be described with very few words.

Putting your problem and analysis into a public issue has several advantages. A developer who takes his project (still) seriously will get a notification about the issue by e-mail anyway. Also he can choose to respond by e-mail and GitHub will add his response to the e-mail, for example requesting further info.

And then there is the big advantage of the GitHub issues being sort of a running list of all known issues. There might be other smart developers who like to use this framework and such they are able to put their mind to it as well if they are so inclined.

So you see that making your issue public is the best thing you can do because it makes it all the more likely to be tackled soon.

Unit Test

A big project like DTCoreText probably has several Unit Tests. In this case the issue has to do with parsing the HTML, which in DTCoreText is done by DTHTMLAttributedStringBuilder. DTCoreText specifically has two unit test targets, one for running tests on iOS (device or simulator) and another one for running native Mac tests. On Mac the focus is to compare the output of DTCoreText with Mac’s native initWithHTML methods.

This is the stage where developers can truly shine and stick out from the crowd of issue-flingers. You will be assured deep respect from any developer if you are able to express your issue in a unit test.  Pros call this “test driven development” (TDD).

At first you create the unit test that it fails where the output does not match your expectation. The place for resources, like an HTML file for this case is Core/Test/Resources named RTL.html. Then I added the following unit test to DTHTMLAttributedStringBuilderTest.m.

- (void)testRTLParsing
{
   NSBundle *bundle = [NSBundle bundleForClass:[self class]];
   NSString *path = [bundle pathForResource:@"RTL" ofType:@"html"];
   NSData *data = [NSData dataWithContentsOfFile:path];
 
   DTHTMLAttributedStringBuilder *builder = [[DTHTMLAttributedStringBuilder alloc] initWithHTML:data options:nil documentAttributes:NULL];
   builder.shouldKeepDocumentNodeTree = YES;
   NSAttributedString *output = [builder generatedAttributedString];
 
   NSUInteger paraEndIndex;
   NSRange firstParagraphRange = [[output string] rangeOfParagraphsContainingRange:NSMakeRange(0, 0) parBegIndex:NULL parEndIndex:&paraEndIndex];
   STAssertEquals(NSMakeRange(0, 22), firstParagraphRange, @"First Paragraph Range should be {0,14}");
 
   NSRange secondParagraphRange = [[output string] rangeOfParagraphsContainingRange:NSMakeRange(paraEndIndex, 0) parBegIndex:NULL parEndIndex:NULL];
   STAssertEquals(NSMakeRange(22, 24), secondParagraphRange, @"Second Paragraph Range should be {14,14}");
 
   CTParagraphStyleRef firstParagraphStyle = (__bridge CTParagraphStyleRef)([output attribute:(id)kCTParagraphStyleAttributeName atIndex:firstParagraphRange.location effectiveRange:NULL]);
   CTParagraphStyleRef secondParagraphStyle = (__bridge CTParagraphStyleRef)([output attribute:(id)kCTParagraphStyleAttributeName atIndex:secondParagraphRange.location effectiveRange:NULL]);
 
   DTCoreTextParagraphStyle *firstParaStyle = [DTCoreTextParagraphStyle paragraphStyleWithCTParagraphStyle:firstParagraphStyle];
   DTCoreTextParagraphStyle *secondParaStyle = [DTCoreTextParagraphStyle paragraphStyleWithCTParagraphStyle:secondParagraphStyle];
 
   STAssertTrue(firstParaStyle.baseWritingDirection==kCTWritingDirectionRightToLeft, @"First Paragraph Style is not RTL");
   STAssertTrue(secondParaStyle.baseWritingDirection==kCTWritingDirectionRightToLeft, @"Second Paragraph Style is not RTL");
}

I previously wrote about how to create Unit Tests, maybe now is a good time to read up on that.

The above unit test loads the HTML file from the octest bundle, generates an attributed string via the string builder, gets the ranges of the two paragraphs and then gets the paragraph style at the range starts. Since we cannot easily inspect the CTParagraphStyle objects which are Core Foundation objects, we convert them to DTCoreTextParagraphStyle instances. Finally we can assert that both should have an RTL writing direction.

The bottommost STAssertTrue fails and that’s good.

Now we can commit these additions referencing the number of the issue on GitHub which will make it show up in the issue once it gets merged into the master project. All you need is to add a hash and the number of the issue to the commit message. If you push these commits to your fork of the project you could even submit it to the project owner via pull request.

Another great thing about having it in writing, that is in a unit test, is that once this “small issue” is fixed then the developer would know the instance when some other unrelated change would break it. As projects become more complex with more and more internal dependencies this becomes more likely.

Fix It If You Can

A good issue (that includes some hints at what might be going on) is a third of the distance. The second leg is the Unit Test. Now finally the third part that separates you from being a programming demi-god is to actually fix the issue.

If you search through this project you will end up in the interpretAttributes method of DTHTMLElement. This interprets HTML attributes that have an impact on the display style. There you can see that based on the value of the HTML attribute dir the baseWritingDirection of _paragraphStyle is set.

And this is the only place where baseWritingDirection was set. The interpretation of the direction CSS style attribute is missing from applyStyleDictionary: which gets called on each parsed DTHTMLElement after the attributes are interpreted. So we add it and of course again reference the issue. Putting a Fixes or Closes in front of it also closes the issue as soon as these changes are pushed to the master repo.

I admit that the fix for this specific issue is ridiculously simple, especially since I know my way around my own code which I also try to make as obvious as possible with many comments in between. Nevertheless in my experience bugs are often fixed in a few lines of code.

But you see another effect from the unit test and aforementioned research: by knowing that it has to do with CSS attributes, knowing when it works and when it fails I was able to immediately know the place in the code where something is missing. In this example the implementation for direction was missing and only present for the dir HTML attribute.

Having fixed this you can verify that you fix didn’t break anything else by again launching the demo app. By again looking at the CurrentTest piece we find that now both paragraphs are right-aligned and inspecting the Ranges we see that both have the correct writing direction value.

Now we run the unit tests (including our new one) and find that all is well.

From this success message we can be reasonably certain that our fix did not have a negative impact on all the other tests. So now we are ready to push that back to our fork and from there let the master repo have it. The detailled steps for that are in the earlier mentioned GitHub Fork, Fix, Pull Request article.

Conclusion

Don’t be lazy! If it turns out that “you are holding it wrong” and your problem is actually home-made then a bit of research will prevent you from making a fool of yourself and alienate the project maintainer. If the issue is repeatable in the demo app of the project in question then it is also worthy of a bug report.

I have explained the three levels towards the apex of contributing to Open Source projects:

1. Provide good bug reports and do some background research
2. Make unit tests
3. Fork, Fix, Pull Request

In the case of DTCoreText I opted to make it Open Source for the simple reason that I by myself could never hope to cover all facets of HTML. Having it open to the public means that I cannot make any serious money from it, in exchange for getting other people’s fixes and contributions.

Even if you never make it beyond level 1, then still that’s no excuse for not writing the very best issue reports that you can.

Now what about issues that stay open for a long time that don’t look like something is happening with them? In all likelihood those are not written well, the project maintainer has too little time available to also do the necessary research or has too little time period.

You have to assume that this person has to work for his money, either by doing contract work or simply being employed. If you were to offer sponsorship for getting certain issues fixed or features implemented you might be astonished how quickly time will be found.

Categories: Recipes