Closed Bug 635522 Opened 9 years ago Closed 9 years ago

[rtl] tab close buttons in the wrong spot

Categories

(Firefox for Android Graveyard :: General, defect, P2)

ARM
Android
defect

Tracking

(fennec2.0+)

VERIFIED FIXED
Tracking Status
fennec 2.0+ ---

People

(Reporter: kbrosnan, Assigned: vingtetun)

Details

(Whiteboard: [4.0b5] [fennec-softblocker][need-doc])

Attachments

(5 files)

Using the Arabic language pack the tab close buttons are completely over the tabs. The edge of the x should be over the right edge of the tab.

Firefox Mobile 4 beta 5 candidate 3
Android 2.2 HTC G2 (Desire Z)
tracking-fennec: --- → ?
Priority: -- → P2
Whiteboard: [4.0b5]
low priority blocker
Assignee: nobody → 21
tracking-fennec: ? → 2.0+
Whiteboard: [4.0b5] → [4.0b5] [fennec-softblocker]
Attached patch Patch (m-c)Splinter Review
The patch add start/end attributes support to <stack/> element. In some cases this make life simpler when dealing with a UI that support RTL.
Attachment #514775 - Flags: review?(enndeakin)
Comment on attachment 514775 [details] [diff] [review]
Patch (m-c)

Good, but please add some tests for left/right/start/end in rtl mode as well.

The idea here is that left and right attributes override the start and end attributes. It would good to have a followup bug to improve this code a bit, such that:
 - it doesn't need to check start/end if left and/or right were specified.
 - it doesn't duplicate so much code
Attachment #514775 - Flags: review?(enndeakin) → review+
Comment on attachment 514776 [details] [diff] [review]
Patch (Front-End)


>       if (Browser.floatedWhileDragging) {

Add a comment here to say: We need to adjust for RTL layout

>-        let [leftVis,,leftW,] = Browser.computeSidebarVisibility();
>-        x = Math.round(Math.max(0, leftW * leftVis));
>+        let [leftVis, rightVis, leftW, rightW] = Browser.computeSidebarVisibility();
>+        let [leftSidebar, rightSidebar] = [Elements.tabs.getBoundingClientRect(), Elements.controls.getBoundingClientRect()];
>+        if (leftSidebar.left > rightSidebar.left)
>+          x = Math.round(Math.max(0, rightW * rightVis));
>+        else
>+          x = Math.round(Math.max(0, leftW * leftVis));
Attachment #514776 - Flags: review?(mark.finkle) → review+
Comment on attachment 515116 [details] [diff] [review]
Patch (m-c) for checkin

Make sure you file the followup bug suggested by Neil in comment 4
Attachment #515116 - Flags: approval2.0+
pushed the front-end fix: http://hg.mozilla.org/mobile-browser/rev/fb597ad4bfa7

Filed bug 637514 as a followup for the code clean-up:
Whiteboard: [4.0b5] [fennec-softblocker] → [4.0b5] [fennec-softblocker][need-doc]
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Build id: Mozilla/5.0 (Android;Linux armv7l;rv:2.0b13pre)Gecko/20110301
Firefox/4.0b13pre Fennec /4.0b6pre 

The close x icon is still in the wrong position. I will reopen the bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Gasp, I've noticed some left-over in tabs.xml before checking and I've remove too much code.
The patch change "left" by "start" into tabs.xml (and add some css rules for the navigation buttons in rtl)
Attachment #515907 - Flags: review?(mark.finkle)
Attachment #515907 - Flags: review?(mark.finkle) → review+
Pushed the followup: http://hg.mozilla.org/mobile-browser/rev/825f90dd0e44
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
verified fixed on build id: Mozilla /5.0 (Android;Linux armv7l;rv:2.0b13pre)
Gecko/20110302
Firefox/4.0b13pre Fennec /4.0b6pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.