515971 - [faceted search] Faceted search should be made RTL-friendly

Status: VERIFIED FIXED

(Thunderbird :: Search, defect)

Description

[David Ascher (:davida)]

This is going to take someone who understands RTL'ing XHTML, and should probably wait until we've actually finalized the LTR layout, to avoid wasted effort.

Comment 1

[David Ascher (:davida)]

Ehsan, it'd be great if you could throw in your input on how we could get the new search results page to work well RTL.  The relevant files are:

http://mxr.mozilla.org/comm-central/find?string=facetView&tree=comm-central&hint=

Comment 2

[(no longer active)]

Do you have some screenshots?

Blake: is this something we can look into some time that you're in the office?

Comment 3

[Blake Winton (:bwinton) (:☕️)]

Attachment: A patch to rtl-enable most of the faceted search tab.

So, we still don't do the date view, cause it's canvas, but this at least makes most everything else work out better.

Ehsan helped me a lot with the patch, so it's probably got an implicit rtl-ui-r=him.

(Philor, please let me know if you don't have the time to review this patch, and I'll find some other sucker^H^H^H^H^H^H friendly developer to review it.  Maybe asuth.  He likes reviewing patches, doesn't he? :)

Thanks,
Blake.

Comment 4

[(no longer active)]

This is not an internationalization bug.

Comment 5

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 449953 [details] [diff] [review]
A patch to rtl-enable most of the faceted search tab.

Looks good.  I'm trying it out now but it feels fine so far.


> .message-subject-group {
>-  padding-left: 2px; /* to lign up the subjects with the "Top N messages out of M" */
>+  -moz-padding-start: 2px; /* to lign up the subjects with the "Top N messages out of M" */
> }

Can you fix that comment typo while you're there

Comment 6

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 449953 [details] [diff] [review]
A patch to rtl-enable most of the faceted search tab.

Well, I think I've been old-review-guilt-tripping philor for long enough, so now it's asuth's turn.  ;)

Later,
Blake.

Comment 7

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 449953 [details] [diff] [review]
A patch to rtl-enable most of the faceted search tab.

The left/right start/end transitions look right, and the other blocks seem safe because of their constrainty things.

Comment 8

[Blake Winton (:bwinton) (:☕️)]

Checked in as http://hg.mozilla.org/comm-central/rev/f753e2711eed

Thanks,
Blake.

Comment 9

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 449953 [details] [diff] [review]
A patch to rtl-enable most of the faceted search tab.

I don't know if we want to take RTL-fixes for 3.1, but I figure it's worth nominating.

Thanks,
Blake.

Comment 10

[Mark Banner (:standard8)]

Do we have any idea how this would affect a) themes, b) extensions if we were to land for 3.1.x branch?

Comment 11

[Andrew Sutherland [:asuth] (he/him)]

I've been operating under the assumption it's impossible to theme or extend the faceted search UI... at the very least, it's quite hard since you can't XUL overlay an html file...

Comment 12

[Dan Mosedale (:dmosedale, :dmose)]

:asuth, well hard to do with static HTML, sure.  But it should be straightforward enough by inserting elements into the live DOM, no?  Of course, whether anyone is actually doing that is really the question...

Comment 13

[Andrew Sutherland [:asuth] (he/him)]

(In reply to comment #12)
> :asuth, well hard to do with static HTML, sure.  But it should be
> straightforward enough by inserting elements into the live DOM, no?  Of course,
> whether anyone is actually doing that is really the question...

Well, things are XBL and lots of things are constantly getting created and re-created.  So one would need to manually poke CSS rules into the HTML page after detecting its existence, etc.

In any event, I think I responded from the wrong angle.  There is basically one  extension point that does not require massive hacky extension work: custom gloda attributes marked for faceting.  Those will continue to work with Blake's patch.  In the off-chance someone has done the massive hacky extension work, I would not expect it to break, and if it did, the change-work required would be very small and easily justified given the RTL improvements.

Comment 14

[Dan Mosedale (:dmosedale, :dmose)]

Yes, this sounds like the right trade-off to me as well.

Comment 15

[Mark Banner (:standard8)]

Comment on attachment 449953 [details] [diff] [review]
A patch to rtl-enable most of the faceted search tab.

Ok, you've convinced me, lets go for it for 3.1.5.

Comment 16

[Blake Winton (:bwinton) (:☕️)]

(I'm still on the NetBook, so I'm marking it checkin-needed for 3.1.5.)

Comment 17

[Mark Banner (:standard8)]

Checked in to 1.9.2: http://hg.mozilla.org/releases/comm-1.9.2/rev/41a87133b218

Comment 18

[Ludovic Hirlimann [:Usul]]

verified with Mozilla/5.0 (Windows; U; Windows NT 5.1; ar; rv:1.9.2.11) Gecko/20101004 Thunderbird/3.1.5