45.75 KB, image/png
118.60 KB, image/png
17.08 KB, image/tiff
14.13 KB, image/png
84.84 KB, image/png
41.85 KB, image/png
43.98 KB, image/png
45.72 KB, application/x-gzip
44.51 KB, application/pdf
80.61 KB, patch
|Details | Diff | Splinter Review|
The search field in the Mac build for Hebrew (126.96.36.199) looks ugly. The right-to-left flip of the toolbar in the Mac doesn't work as well as in Windows because the search field is rounded in the Mac. You can also see that tabs need improvement. STR: 1. Install 2.0x Hebrew build on Mac. Expected: Layout of search field should display correctly (just right-to-left) Actual: The search field field looks made up of three separate elements.
In the screenshot, it seems that all of the UI elements that should be reversed in RTL locale are not reversed.
Sorry for the lame question, but are you using the default theme?
I can confirm this with an English (US) Mac OS X 10.2, running Fx 188.8.131.52 with a new profile and the default theme.
(In reply to comment #3) > Sorry for the lame question, but are you using the default theme? > Pinstripe is not RTL-compatible.
So, Asaf, this is not a regression? It didn't work in 1.5 as well? This problem renders Arabic and Hebrew Mac Firefox almost unusable, we should either fix the theme (maybe if we can't make Pinstripe RTL-friendly, consider using Winstripe for Mac RTL builds) or pull Mac ar and he builds from the downloads page (like we did e.g. with gu-IN)...
(In reply to comment #7) > So, Asaf, this is not a regression? It didn't work in 1.5 as well? Right.
I startes working on a fix for the pinstripe theme in Bug 221824 (attachment 205080 [details] [diff] [review]). if you have the time and knowledge feel free to finish it. (the patch may be a bit outdated because of changes in the theme for Firefox 2.0)
These issues persist in the new Proto theme. I am requesting blocker status for this bug. The same issues in gnomestripe (bug 406330) are blocking-firefox3+. It seems to me that the default Mac theme deserves the same treatment.
This is not a blocker for Fx3 as we have never supported RTL on Mac. We have always supported RTL on Linux, which is why that bug blocks.
Per comment #230 in bug #397723, it seems to me that this bug should block 397723. Could someone with appropriate privileges make this happen?
(In reply to comment #13) > Per comment #230 in bug #397723, it seems to me that this bug should block > 397723. Could someone with appropriate privileges make this happen? Done.
I think that
Oops. I think that this is orthogonal to the new theme. We should look at making it RTL compatible.
Mano: apparently proto makes this a lot worse; did many people even try to use Mac builds? Just trying to figure out if this is a regression ...
I don't know what do you mean by "worse", it's as non-supported in both themes. I don't think we should be shipping RTL builds on mac, they were never usable
Created attachment 308521 [details] rendering of navigational graphic in Mac platform on RTL language - ar this is an image i captured on my mac osx 10.4 platform for the beta 4 build of Arabic here is a link to an image captured by tchung for hebrew beta 4 build http://img391.imageshack.us/my.php?image=hetoolbarjr1.png
Asaf: Is it a rendering issue? I guess not, and if so we should at least try to solve it.
Created attachment 308523 [details] Image of Beta 4 build of arabic on Mac 10.4 platform trying this attachment again, apologies
Created attachment 308526 [details] full screen shot of Beta 4 arabic build on Mac 10.4 platform full screen view of arabic demonstrating rendering issue on Mac
I know this is late in the game, but to add my 2 cts: Fact is that we shipped RTL localizations on the mac on firefox 2. They might not have looked perfect, or had the weirdest hacks in intl.css, but in the end, we shipped something. Ending up with something totally non-shippable would be a regression to me.
Did we hack intl.css? As far as I can tell it was as broken.
I've been using RTL Mac on OS X and Linux since 2004. As I recall, this more or less the sequence of events 1) Firefox 1.0 had no RTL support built in, but you could install a RTL version of the classic theme (maybe also modern) that was created by the Hebrew localization team. I think this was a descendant of the corresponding navigator theme. 2) Firefox 1.5 had RTL support in winstripe and gtkstripe but not pinstripe. Pinstripe looked more or less the same as in 1.0. The back and forward buttons were reversed, but most of the other glitches were relatively minor 3) Visual Referesh in Firefox 2.0. These actually makes pinstripe look even worse than in Firefox 1.5 . Tabs rounded the wrong way, various other aspects. It's so bad installing the Ubuntu Human theme (allegedly designed for Win and Linux) actually improves the experience and becomes my default theme. Summarizing and updating my observations from comment #116 to bug #397723, here are the most glaring issues based on my (admittedly light) use of recent betas: 1) All buttons in the toolbar (forward/back/history) are still in the LTR orientation. 2) The search-engine icon and search button are rounded towards rather than away from the search field. (I guess this is the same as #1.) 3) Tab bar: the corners are rounded towards rather than away from the tab itself. 4) URL bar: this actually has a complete RTL orientation, except that is wrong, as all (or almost all) urls are in English. It should be favicon on the left, url left-justified and rendered as LTR text, the drop down list on the right. In the screenshot (attachment #292197 [details], created for proto 0.8 but still accurate except for the new history buttons) you'll notice that the trailing slash appears on the left of the url rather than the right, the tell-tale sign of English text rendered as RTL instead of LTR. I think all of 1-3 should be easy to fix, as I recall there are forward-rtl, back-rtl, etc. icons that can be set. (I haven't been able to find docs on theme changes in FF3, at least as affects RTL. But clearly you guys are the experts so I'll defer to your judgment.) I think a simple horizontal reflections to create new icons will suffice (or id swap, for cases where the icon pairs are symmetric). For 4, I'm not sure what's involved, but gtkstripe does it correctly by default so I think the ability should be built-in to gecko. Finally, let me just say thanks for finally taking a look at this. I'm happy to contribute testing---just add a comment telling me to grab the latest nightly. I'm not an expert on mozilla code, but I can apply patches and build from source, so if there is something would like to test before a commit I can do that also. Oh, I remember encountering difficulty in figuring out how to do a "RTL build." So instructions on that would be appreciated if you want me to test a patch.
Created attachment 309927 [details] Image of the navigation bar in FF3 PreBeta5 Hebrew on Mac OSX 10.4.10 Search field is highlighted. Also note the RTL layout of the URL causes the last forward slash of the URL to appear in front of the "http:" prefix
> Also note the RTL layout of the URL causes the last forward slash of the URL to > appear in front of the "http:" prefix We (fa, he, etc) fix this in intl.css by setting the direction and alignment of the address bar. Please report it as a bug for the locale.
(In reply to comment #28) > We (fa, he, etc) fix this in intl.css by setting the direction and alignment of > the address bar. Please report it as a bug for the locale. > I prefer to get rid of any intl.css 'hacks'. If the URL bar is *always* LTR, it should be mentioned in the chrome, and not require each locale to specify it manually. Same should also be the guidelines for extensions - If the extension is not available for the RTL locales, and fallback to English but stays in RTL layout, which most of the time make the interface very complicated and non clear. Currently we [he] have rule in intl.css for adblock inside the shipped locale package, and I hope to get it removed one day.
Created attachment 310099 [details] Screenshot of navigation bar on Heberw Mac (top) and Hebrew Linux (bottom) Notice a small bug in the Linux address area, but otherwise represents the expected behavior for the Mac version. Of course, Mac's rounded buttons require more work.
Does this block the B5 release of RTL locales on Mac?
As a start point, can you please give us some basic information about the proto theme? Does it have some kind of shading on the buttons that may make it difficult to mirror the theme?
axel, per comment #31, we had been discussing that in email with Tomer and Ayman and l10n drivers - we are trying to assess if it would be a blocker. the user experience is very bad and it seems Mac usage in general in these areas may be lower. we did block it for beta 4. if the user experience doesn't improve then it may be that we'd do same for b5. thoughts?
Update: Thanks to a Yehuda (from comment #30) we now have Firefox Proto theme with basic RTL support (the main navigation toolbar at the moment). We'll review it and post it here. It may be too harmful to commit directly to the tree without reviewing, so please pay the required attention. Expect RTL Mac theme in the very soon future, I hope we will be on time for RC1.
Created attachment 312641 [details] Work in progress (The main navigation toolbar is quite usable!) I've reviewed the changes on top of my Linux box. I most admit that Yehuda is doing wonderful job, and there is a huge progress toward resolving this bug.
Are there version of theme available (or can be made available)? I'd like to do some testing on on Mac (currently running Tiger).
Created attachment 312687 [details] Mac theme RTL overrides Attached is Yehuda's fixes I've reviewed yesterday. It is not final yet, but if you are testing it please publish you thoughts and bugs here.
I'm not having much success. Perhaps I'm doing it wrong. I downloaded attachment 312687 [details]. I then copied the contents of classic/global to toolkit/themes/pinstripe/global, and I copied the contents of classic/browser into browser/themes/pinstripe/browser and rebuilt. The result is that the URL bar is now rounded correctly and the url text is left-justified, but I have no forward or back buttons, and the rounded edges of the search bar are entirely missing. I'm not sure if this is and problem with my build or if its an issue still not fixed, but the corners on the tabs are still rounded the wrong direction.
(In reply to comment #38) First of all, thanks for checking this patch. The search edges bug and the missing back and forward arrows can be explained by the build process you used. The patch was intended to be applied after a successful build (or even on top of a downloaded nightly build). I guess that your process failed because the final classic.jar did not include the new *-rtl images. I need to figure out how to do that. On top of that, there was a change in browser.css and browser.xml on Mar 31, which I was not aware of until now. If you do a checkout and build just before applying my patch, it can cause further issues with the back and forward buttons. I will post an updated patch later today or tomorrow which should be compatible with the latest changes in the CVS. The tabs' rounding are still under construction and were not meant to be included in this patch - this time it is not a CVS conflict or build issue. Hopefully they will be included in this later patch as well.
(In reply to comment #38) > I then copied the contents of classic/global to > toolkit/themes/pinstripe/global, and I copied the contents of classic/browser > into browser/themes/pinstripe/browser and rebuilt. As this is not a real patch, you'll need to extract it manually or add the attachment files into the classic.jar. At the moment the files are not on the CVS, and I've not requested any review flag on the file, as it is still incomplete. At the moment it will be complete or at least near that point, I'll start patching the files and request the required reviews. I don't think it will be ready before RC1 freeze, but I hope it will be ready before RC2.
Putting some Mikes on CC to get them into the loop here, now that there is action.
When I didn't find any directories named skin in Minefield.app, I assumed I needed to build from source. I now have it running with the 3/26 nightly. Since I haven't used the trunk too much, I'm not sure how much of the following is theme specific and how much is toolkit bugs, but I've noticed the following. (Undoubtedly some of these things are in the process of being fixed) 1) I can't seem to access any bookmarks from the bookmarks menu. I'm fairly certain this was _not_ a bug before I patched the theme. The only bookmark in the menu is "Bookmarks Toolbar", but it appears empty in the menu, even though I can see and use bookmarks in the actual toolbar. 2) In bookmarks organizer, the forward/back buttons are backwards and the rounded edge on the is on the wrong side of the drop-down buttons. 3) If the search field is selected and I click on the search-engine selection button (or whatever its actual name is), the highlight disappears from around that button. I remains around the rest of the search field, however. 4) This may be a bug in Proto generally, but the preview image in the Theme Manager looks remarkably identically to the one from pinstripe and not look how the theme actually looks now. 5) In the add-ons manager generally, the little add-on icon (which is on the right, at least in RTL mode), is cut half. It appears only the right-half is shown. Resizing the window fixes the issue, but clicking on another tab in the manager re-intrdouces the cut off. I don't have time to create screen-shots just now, but if people want them I can do that tonight. If I need to open new bugs, let me know. Even though the theme is incomplete, this is much better than what we had a week ago. Keep up the good work!
OK, we can scratch #1. I don't what the problem was, but after a restart my bookmarks menu is working again. The other bugs have persisted.
Created attachment 312947 [details] Another set of rtl-override files This is another work-in-progess set of files for testers. It includes compatibility fixes with the latest changes in browser.css code, as well as fixes for the tabs display. To test this set: Download the latest nightly (Apr 1st or later); change directory to Contents/MacOS/chrome; extract content of classic.jar; extract this set to override/add the files; and then re-pack classic.jar (including added files). Restart Firefox to see the changes.
(In reply to comment #42) > > 1) I can't seem to access any bookmarks from the bookmarks menu. Not repeatable as noted > 2) In bookmarks organizer, the forward/back buttons are backwards and the > rounded edge on the is on the wrong side of the drop-down buttons. This issue is on my to-do list. > 3) If the search field is selected and I click on the search-engine selection > button (or whatever its actual name is), the highlight disappears from around > that button. I remains around the rest of the search field, however. This issue is repeatable in the original theme. Currently, out of my scope. > 4) This may be a bug in Proto generally, but the preview image in the Theme > Manager looks remarkably identically to the one from pinstripe and not look how > the theme actually looks now. Ditto > 5) In the add-ons manager generally, the little add-on icon (which is on the > right, at least in RTL mode), is cut half. It appears only the right-half is > shown. Resizing the window fixes the issue, but clicking on another tab in the > manager re-intrdouces the cut off. I could not reproduce this one. Can you add a screenshot? Once again, thanks for testing
(In reply to comment #44) I prefer to extract the classic.jar and than change the manifest file to point to the extracted files instead. This remove the need to repack classic.jar again after each change.
Created attachment 313187 [details] [diff] [review] CSS patch You'll need the following files from the previous zip file in order to get a usable RTL theme - cvs diff: browser/themes/pinstripe/browser/Toolbar-rtl.png is a new entry, no comparison available cvs diff: browser/themes/pinstripe/browser/urlbar/endcap-focused-rtl.png is a new entry, no comparison available cvs diff: browser/themes/pinstripe/browser/urlbar/endcap-rtl.png is a new entry, no comparison available cvs diff: browser/themes/pinstripe/browser/urlbar/startcap-active-rtl.png is a new entry, no comparison available cvs diff: browser/themes/pinstripe/browser/urlbar/startcap-focused-rtl.png is a new entry, no comparison available cvs diff: browser/themes/pinstripe/browser/urlbar/startcap-rtl.png is a new entry, no comparison available cvs diff: toolkit/themes/pinstripe/global/menu/menu-arrow-dis-rtl.gif is a new entry, no comparison available cvs diff: toolkit/themes/pinstripe/global/menu/menu-arrow-hov-rtl.gif is a new entry, no comparison available cvs diff: toolkit/themes/pinstripe/global/menu/menu-arrow-rtl.gif is a new entry, no comparison available This is also a friendly request to mark the patch as blocking RC1 or something. :-)
Created attachment 313265 [details] screen shot of distortion of icons in Add-On Manager in Proto RTL I've tested the Add-on manager with the latest nightly and update RTL patch. The behaviour has changed somewhat. First, it has become intermittent. Second, I no longer need to resize the window in order to fix. Usually switching to a different tab will fix it. Third, sometimes half the icons are missing, and sometimes their distorted. Here's a picture of them being distorted. If succeed in reproducing the half of the icons are missing situation, I'll upload a picture of that also.
(In reply to comment #48) > Created an attachment (id=313265) [details] > screen shot of distortion of icons in Add-On Manager in Proto RTL > Thanks for posting the screen shot. Nevertheless, I could not reproduce this issue...
Created attachment 313450 [details] [diff] [review] patch Changelog from previous patch: Fixed images in the RTL toolbar browser.xml fixed to send chromedir parameter to the browser.css Please remoember that you can't build the RTL theme from this patch, and will need also to add the images as described in comment 47.
Comment on attachment 313450 [details] [diff] [review] patch Need review first before asking for approval.
Mass-assigning the new rtl keyword to RTL-related (see bug 349193).
Please note that we are still looking for a review... Yehuda - As I remember this is still not the final patch. Do you remember what is still missing?
Comment on attachment 313450 [details] [diff] [review] patch The patch is incomplete and has a number of issues (some the result of the LTR flavor of pinstripe). I'm currently writing a new version so this has a chance of landing for RC1.
(In reply to comment #53) > Yehuda - As I remember this is still not the final patch. Do you remember what > is still missing? Work on the places manager is still in progress. And, as Ryan noted, changes to the LTR theme (mainly due to bug #427555) should be applied to this patch as well.
Ryan: thanks - a question, though. Would it be worth landing the lion's share of this which *does* work, in order to get the RTL theme into a state where it's shippable? I'm willing to accept some amount of visual error, especially on less seen bits of the UI, and we can then fix the rest as follow ups if time permits.
Created attachment 315034 [details] [diff] [review] Patch v2 This covers everything I could see that can be fixed in the theme. I was hoping bug 427935 would land first, but it'll be easy enough to update this once it does.
i've nominated this for blocking, if it's at all reasonable to fix this for fx3 it would be great for hebrew and arabic (or RTL) languages on Mac platform. it's been an outstanding issue for a while and it would be wonderful if we could address it.
Nice to have, not blocking. That said, I'll try to review tonight.
Comment on attachment 315034 [details] [diff] [review] Patch v2 r=me, but please unbitrot after kevin lands the last theme drop tonight
Comment on attachment 315034 [details] [diff] [review] Patch v2 !renztleb=a
Created attachment 317253 [details] [diff] [review] As checked in mozilla/toolkit/themes/pinstripe/global/menu/menu-arrow-dis-rtl.gif 1.1 mozilla/toolkit/themes/pinstripe/global/menu/menu-arrow-hov-rtl.gif 1.1 mozilla/toolkit/themes/pinstripe/global/menu/menu-arrow-rtl.gif 1.1 mozilla/toolkit/themes/pinstripe/global/console/console.css 1.14 mozilla/toolkit/themes/pinstripe/global/dialog.css 1.5 mozilla/toolkit/themes/pinstripe/global/findBar.css 1.13 mozilla/toolkit/themes/pinstripe/global/jar.mn 1.44 mozilla/toolkit/themes/pinstripe/global/listbox.css 1.7 mozilla/toolkit/themes/pinstripe/global/menu.css 1.15 mozilla/toolkit/themes/pinstripe/global/menulist.css 1.16 mozilla/toolkit/themes/pinstripe/global/radio.css 1.7 mozilla/toolkit/themes/pinstripe/global/tree.css 1.17 mozilla/toolkit/themes/pinstripe/mozapps/extensions/extensions.css 1.47 mozilla/browser/components/places/content/places.xul 1.132 mozilla/browser/components/search/content/search.xml 1.125 mozilla/browser/themes/pinstripe/browser/Toolbar-rtl.png 1.1 mozilla/browser/themes/pinstripe/browser/browser.css 1.149 mozilla/browser/themes/pinstripe/browser/browser.xml 1.11 mozilla/browser/themes/pinstripe/browser/jar.mn 1.86 mozilla/browser/themes/pinstripe/browser/searchbar.css 1.24 mozilla/browser/themes/pinstripe/browser/places/menubutton-end-pressed-rtl.png 1.1 mozilla/browser/themes/pinstripe/browser/places/menubutton-end-rtl.png 1.1 mozilla/browser/themes/pinstripe/browser/places/menubutton-start-pressed-rtl.png 1.1 mozilla/browser/themes/pinstripe/browser/places/menubutton-start-rtl.png 1.1 mozilla/browser/themes/pinstripe/browser/places/organizer.css 1.14 mozilla/browser/themes/pinstripe/browser/places/places.css 1.25 mozilla/browser/themes/pinstripe/browser/urlbar/endcap-focused-rtl.png 1.1 mozilla/browser/themes/pinstripe/browser/urlbar/endcap-rtl.png 1.1 mozilla/browser/themes/pinstripe/browser/urlbar/startcap-active-focused-rtl.png 1.1 mozilla/browser/themes/pinstripe/browser/urlbar/startcap-active-rtl.png 1.1 mozilla/browser/themes/pinstripe/browser/urlbar/startcap-focused-rtl.png 1.1 mozilla/browser/themes/pinstripe/browser/urlbar/startcap-rtl.png 1.1 mozilla/toolkit/components/console/content/console.xul 1.16
The he build looks good using Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.5; he; rv:1.9pre) Gecko/2008042404 Minefield/3.0pr.
Marcia, does it mean we can mark this bug as verified?
This bug was fixed for Firefox 3 a while ago. Lets mark it verified.