Closed Bug 1224129 Opened 4 years ago Closed 4 months ago

Fix direction of boxmodel margin, size and element size in RTL locales

Categories

(DevTools :: Inspector, defect, P3)

defect

Tracking

(firefox42 wontfix, firefox43 wontfix, firefox44 wontfix, firefox45 wontfix, firefox49 wontfix, firefox-esr38 wontfix, firefox50 wontfix, firefox51 wontfix, firefox52 wontfix, firefox68 wontfix, firefox69 wontfix, firefox70 verified)

VERIFIED FIXED
Firefox 70
Tracking Status
firefox42 --- wontfix
firefox43 --- wontfix
firefox44 --- wontfix
firefox45 --- wontfix
firefox49 --- wontfix
firefox-esr38 --- wontfix
firefox50 --- wontfix
firefox51 --- wontfix
firefox52 --- wontfix
firefox68 --- wontfix
firefox69 --- wontfix
firefox70 --- verified

People

(Reporter: magicp.jp, Assigned: itiel_yn8)

References

(Blocks 1 open bug)

Details

(Whiteboard: [btpp-backlog])

Attachments

(6 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20151111030207

Steps to reproduce:

1. Run any versions Firefox in RTL locales (e.g. Arabic)
2. Open any sites (e.g. about:home)
3. Open devtools > Inspector > Box Model
4. Select any elements in HTML pane
5. Confirm inspector-element-annotation and Box Model header


Actual results:

inspector-element-annotation and Box Model header have wrong direction.


Expected results:

inspector-element-annotation and Box Model header should be reverse direction. Please refer to attached image.
Blocks: dt-rtl
Has STR: --- → yes
Component: Untriaged → Developer Tools: Inspector
OS: Unspecified → All
Hardware: Unspecified → All
Triaging (filter on CLIMBING SHOES).
Priority: -- → P3
Whiteboard: [btpp-backlog]
Revised STR and expected results
> rtl- inspector-element-annotation-and-box-model-header-have-wrong-direction-in-inspector.png
revised-expected-results-bug-1224129.png

> 2. Open any sites (e.g. about:home)
2. Open about:home
> 3. Open devtools > Inspector > Box Model
3. Open DevTools > Inspector > Computed
> 4. Select any elements in HTML pane
4. Select #searchSubmit and #searchSuggestionTable


> Actual results:
> inspector-element-annotation and Box Model header have wrong direction.
boxmodel margin and element size have wrong direction.

> Expected results:
> inspector-element-annotation and Box Model header should be reverse direction. 
.boxmodel-margin, .boxmodel-size and #boxmodel-element-size direction should be LTR.
(.boxmodel-padding and .boxmodel-border are unaffected because cannot set negative value)
Summary: [RTL][devtools] inspector-element-annotation and box model header have wrong direction in RTL locales → Fix direction of boxmodel margin, size and element size in RTL locales
FYI, id and class name are different by Firefox version as below.

Firefox 50:
.layout-margin, .layout-size, #layout-element-size

Firefox 51 and later:
.boxmodel-margin, .boxmodel-size, #boxmodel-element-size
Attached file Testcase
Assignee: nobody → tomer.moz.bugs
Status: NEW → ASSIGNED
The patch above fix the following issues:
* content size (width×height)
* border/margin/padding negative values
* box size

Left to do:
* inspection element tooltip is still reversed (height×width)
(In reply to Tomer Cohen :tomer from comment #6)
> * inspection element tooltip is still reversed (height×width)
It seems that it is working well without applying the patch, so this patch is ready. Please let me know if there is yet another location that the box model data is displayed; We have fixed the display of the computed styles panel in bug 1325539.
Attachment #8828947 - Flags: review?(jdescottes)
Comment on attachment 8828947 [details]
Bug 1224129 - Fix direction of boxmodel margin, size and element size in RTL locales

https://reviewboard.mozilla.org/r/106168/#review107466

Thanks Tomer, this fixes the main problem but I have a small issue with the position of the element-size label (see screenshot attached before this review).

I think we could work around it with the following updates to boxmodel.css:

> #boxmodel-element-size {
>   justify-self: flex-start;
>   direction: ltr;
> }
> 
> #boxmodel-position-group {
>   flex: 1;
>   display: flex;
>   align-items: center;
>   justify-content: flex-end;
> }
Attachment #8828947 - Flags: review?(jdescottes) → review+
Since the boxmodel has been revised by bug 1333561, I think we would deprecate the patch above and start again. 

@jdescottes, what do you suggest? (I'm facing automerge issues with this patch, so this should be r- until a rebase anyway)
Flags: needinfo?(jdescottes)
It depends how soon we can replace the old boxmodel in favor of the react-based one.
Gabriel: do we have a bug tracking the removal of the deprecated boxmodel?

If both versions are still around for some time then we should probably fix the RTL display issue in both ...
Flags: needinfo?(jdescottes) → needinfo?(gl)
Looks like this is Bug 1336198. Let's block this bug on it.
Tomer, if you'd like to port your patch to the new box-model widget, you can test it in the layout view (to enable in about:config, using devtools.layoutview.enabled)
Depends on: 1336198
Flags: needinfo?(gl)
Product: Firefox → DevTools
magicp, I believe this is now fixed? The boxmodel in Nightly looks very similar to attachment 8806975 [details].
Flags: needinfo?(magicp.jp)
(In reply to Itiel from comment #13)
> magicp, I believe this is now fixed? The boxmodel in Nightly looks very
> similar to attachment 8806975 [details].

Negative number is not fixed. Why did you believe this is now fixed?
Flags: needinfo?(magicp.jp)
(In reply to magicp from comment #14)
> (In reply to Itiel from comment #13)
> > magicp, I believe this is now fixed? The boxmodel in Nightly looks very
> > similar to attachment 8806975 [details].
> 
> Negative number is not fixed. Why did you believe this is now fixed?

Oh okay, thanks. Didn't notice that.

This bug has not been updated in the last 3 months. Resetting the assignee field.
Please, feel free to pick it up again and add a comment outlining your plans for it if you do still intend to work on it.
This is just trying to clean our backlog of bugs and make bugs available for people.

Assignee: tomer.moz.bugs → nobody
Status: ASSIGNED → NEW
Assignee: nobody → itiel_yn8
Status: NEW → ASSIGNED
Pushed by gluong@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/771185e8e4dc
Force LTR on boxmodel items text r=gl
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.