Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Customized brat tool to include support for Right to Left languages. #1150

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

habibask
Copy link

We have been using brat for LTR text for a while now and then we had a requirement for the tool to work with RTL text. As highlighting RTL annotations was an issue everyone was facing, we had to look for a work-around. After going through the issue logs in brat repo, we found out that this guy (username:)fsalotaibi had already fixed this in his website(http://www.ebsar.com/brat/#/FGANER/109-out).
Issue: #774

We took reference from his client side scripting and made appropriate changes to the server side to work with such model. The highlighting basically depends on start and end positions of the selected text rather than have the fixed offsets for each word even before they are highlighted. The text in the selected doc now flows from the right margin. These changes look better when run in Mozilla firefox than Chrome. In chrome, there is this difficulty of selecting the text with mouse which was not a high priority concern for us. We would be more than happy if anyone can suggest us on how we get around this issue.

Thanks to brat project team and fsalotaibi who are the main sources of this work.

@spyysalo
Copy link
Member

Dear @habibask , many thanks for your efforts!

As you perhaps know, brat is distributed under the MIT license, and contributions considered for merging to the master branch should follow this license.

To the best of my knowledge, @fsalotaibi has not released his changes to brat (see #774 (comment)). Could you please clarify the license status of the code that "[takes] reference from his client side scripting"? Have you been in contact with the author?

@reckart
Copy link

reckart commented Sep 18, 2015

Wow, this looks like a pretty heavy patch.

I looked at the changes from @fsalotaibi before doing the RTL support in WebAnno but then decided to start from scratch. I didn't really understand what kind of things he did with the SVG adding additional empty spans with manual direction overrides. It looked kind of strange to me. But I'm not neither a Javascript not a SVG guru...

For WebAnno RTL support, I didn't have to change anything in the WebAnno backend except sending an additional direction flag to the frontend.

In the frontend, I also updated jquery and jquerysvg, but the rest of the changes were only to visualizer.js.

@habibask
Copy link
Author

Hi @spyysalo.

I would like to clarify that we are not looking for this branch to be merged with master as we don't think the changes here are compatible with the main repository that supports LTR text. We just wanted to make this available for others who are still looking to host an RTL version of BRAT.

Regarding the MIT license, in my earlier commit I had included a GPL version of a third-party-library sprintf.js which I have reverted back to the old file now. Except for this, every other change still complies with MIT license. And we did try contacting @fsalotaibi 4 weeks ago to ask for access to his instance of brat or for the changes and haven't heard back.

Please let me know if you have any other questions. Thanks!

@spyysalo
Copy link
Member

we are not looking for this branch to be merged with master

OK, thank you for the clarification, I agree that this is best. We may still want to look for the best way to make this available as a pull request might be a bit difficult for new users! Many thanks again for your contribution, it's very good to have a version that supports RTL!

amadanmath pushed a commit that referenced this pull request Mar 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants