Created attachment 8822493 [details] Screenshot Steps to reproduce: a. Open inspector on an RTL Firefox build. b. Right click on any rule and choose to open the MDN documentation from the context menu. Expected result: Since these documents are not available on Hebrew locale, documentation should be displayed as LTR. We have a similar issue for the regular MDN; if the popup is an iframe, it might be more suitable to fix this issue on the MDN side.
Inspector bug triage, filter on CLIMBING SHOES. For the moment we always use the en-US version of MDN for this popup, so it would make sense to force the content to use dir=ltr here. The popup content is created in CssDocsTooltip.js . Adding the attribute "dir" set to "ltr" should fix it here. Should be a good first bug. To test RTL direction with a local build of devtools, the easiest way is to use the ForceRTL add on (https://addons.mozilla.org/en-US/firefox/addon/force-rtl/).  http://searchfox.org/mozilla-central/rev/225ab0637ed51b8b3f9f4ee2f9c339a37a65b626/devtools/client/shared/widgets/tooltip/CssDocsTooltip.js#77-82
Hi, can I try on this bug?
Towkir, are you interested in this bug?
:ntim sorry to override you here, but I meant to reply to nnn_bikiu0707 first. They showed interest in this bug first, so it's fair to assign it accordingly :) nnn_bikiu0707: Thanks for wanting to fix this! I gave a few pointers in comment 1. If this is your first contribution, have a look at our contribution guides: - https://developer.mozilla.org/en-US/docs/Tools/Contributing - https://wiki.mozilla.org/DevTools/Hacking You can reach us on IRC in the #devtools channel if you have any question. Or simply comment on this bug, and if you have a question don't forget to set a "Need information" flag using the form below. This way you will be sure we get a notification about your comment.
Created attachment 8827313 [details] [diff] [review] fix_bug_1326265.patch Hi :jdescottes], thank you for letting me work on this bug. I did what you mentioned in comment 1. Let me know if something needs changing.
Comment on attachment 8827313 [details] [diff] [review] fix_bug_1326265.patch Review of attachment 8827313 [details] [diff] [review]: ----------------------------------------------------------------- That fixes the issue, thanks! I now think it would be better to do this in the constructor of MdnDocsWidget.js  The reason is that we can "force" ltr because we use "https://developer.mozilla.org/en-US/docs/Web/CSS/" as the base URL for getting the content from MDN and this is all done in MdnDocsWidget. Can you also add a small comment explaining that rtl can be forced because we use the en-US version of MDN? Sorry for changing my mind about this, but it should be easy to adapt your current patch!  http://searchfox.org/mozilla-central/source/devtools/client/shared/widgets/MdnDocsWidget.js#244-279
Created attachment 8827462 [details] [diff] [review] add_ltr_mdndocswidget.patch No problem. I added some lines to the MdnDocsWidget.js. Let me know what you think.
Comment on attachment 8827462 [details] [diff] [review] add_ltr_mdndocswidget.patch Review of attachment 8827462 [details] [diff] [review]: ----------------------------------------------------------------- Perfect thanks! I will submit your changeset to try, which is our continuous integration platform. This will run our test suite on a various set of platforms, to make sure the change didn't introduce unexpected regressions. If the tests are ok we will then ask to a sheriff to land your changeset on one of our integration branches. It will then "ride the trains" from Nightly to release and will ship with Firefox 53.
Created attachment 8827472 [details] [diff] [review] add_ltr_mdndocswidget.amend.patch I just added r=jdescottes to your commit message so that we can keep track of the reviewer after this lands!
Here is the link to the try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=646669e7f04e2d6ff9cdc8622b33079af57c624c This page will be updated with test results as soon as the test suites are completed.
(In reply to Julian Descottes [:jdescottes] from comment #10) > Here is the link to the try run: > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=646669e7f04e2d6ff9cdc8622b33079af57c624c > > This page will be updated with test results as soon as the test suites are > completed. Thank you for your help. I looked at the page and it showed some tests failed. What should we do?
(In reply to Bao Quan [:beekill95] from comment #11) > (In reply to Julian Descottes [:jdescottes] from comment #10) > > Here is the link to the try run: > > https://treeherder.mozilla.org/#/ > > jobs?repo=try&revision=646669e7f04e2d6ff9cdc8622b33079af57c624c > > > > This page will be updated with test results as soon as the test suites are > > completed. > > Thank you for your help. I looked at the page and it showed some tests > failed. What should we do? None of the failures are related to your patch. Congrats on your first patch :) I'm going to set checkin-needed and it should land in Nightly in about 2 days.
Thank you guys :)
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4be42c91eaa2 Set attribute dir to ltr to fix MDN article snippets appear in RTL. r=jdescottes