Symmetric swapping problem on Mac

RESOLVED FIXED in mozilla0.9.8

Status

()

Core
Layout: Text
RESOLVED FIXED
17 years ago
10 years ago

People

(Reporter: Frank Tang, Assigned: Frank Tang)

Tracking

Trunk
mozilla0.9.8
PowerPC
Mac System 9.x
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments, 2 obsolete attachments)

(Assignee)

Description

17 years ago
this is a place holder for now. not confirm yet.
(Assignee)

Comment 1

17 years ago
Created attachment 62853 [details]
test cases
(Assignee)

Comment 2

17 years ago
Created attachment 62854 [details]
correct display should be
(Assignee)

Comment 3

17 years ago
no problem on Linux
no problem on Japanese Window NT
Status: NEW → ASSIGNED
(Assignee)

Comment 4

17 years ago
Created attachment 62855 [details]
problem on MacOS9
(Assignee)

Comment 5

17 years ago
on Mac OS 9, after taking the patch from bug 116976, bug 116982, and bug 117028
I still see this problem . The Symmetric Swapping is wrong. 
need to test on Mac OS X
Created attachment 62857 [details]
Test of all unicode characters with the mirroring property

Updated

17 years ago
Attachment #62855 - Attachment mime type: text/plain → image/jpeg
(Assignee)

Comment 7

17 years ago
Created attachment 62917 [details] [diff] [review]
patch v1
(Assignee)

Comment 8

17 years ago
ok, another problem of casting 32 bits address to 16 bits data address
and big endian issue.

This patch work on win32 and MacOS X. Need to try MacOS 9 and Linux.
(Assignee)

Comment 9

17 years ago
look like this bug will also impact other big endian unix
(Assignee)

Comment 10

17 years ago
this patch won't fix MacOS 9. The reason is we believe MacOS 9 handle Hebrew
Reverse so the sym swapping code does not apply to it. 
We probably should put sym swaping code in the mac gfx when the direction is set
to RTL
(Assignee)

Comment 11

17 years ago
It is too hard to fix MacOS 9. I propoes we turn off the Hebrew ordering code on
MacOS9 and use the same code path as MacOS X to do the job.
(Assignee)

Comment 12

17 years ago
Created attachment 62920 [details] [diff] [review]
additional mac gfx change to fix MacOS 9 problem
(Assignee)

Comment 13

17 years ago
both patches in this bug are valid. We need the "patch v1" to fix MacOS X and
other big endian platform.
we also need "additional mac gfx change to fix MacOS 9 problem" to address other
bidi issues. 
(Assignee)

Updated

17 years ago
Blocks: 117104
(Assignee)

Comment 14

17 years ago
I have not test 

http://bugzilla.mozilla.org/attachment.cgi?id=62920&action=view

on MacOS X yet. It should work. Let me try tomorrow.
(Assignee)

Comment 15

17 years ago
I believe the following three patch should fix most mac os hewbrew problem

big endian issue in nsFrame.cpp
see latest patch in http://bugzilla.mozilla.org/show_bug.cgi?id=116976

big endian issue in nsBidiImp.cpp 
http://bugzilla.mozilla.org/attachment.cgi?id=62917&action=view

turn off hebrew reordering and do not report hint on mac gfx
http://bugzilla.mozilla.org/attachment.cgi?id=62920&action=view
(Assignee)

Updated

17 years ago
Attachment #62920 - Attachment is obsolete: true
(Assignee)

Comment 16

17 years ago
Created attachment 62998 [details] [diff] [review]
mac gfx patch which solve MacOS 9/X Hebew/Arabic at once.

This patch turn off all bidi & shaping support on mac.
For Hebrew:
on macOS 9, it turn off reverse and use QD to display
on macOS X, it use ATSUI to display
For Arabic:
on both mac OS 9 / X , we turn off QD for arabic
we only use ATSUI to display Arabic.
(Assignee)

Updated

17 years ago
Attachment #62998 - Attachment is obsolete: true
(Assignee)

Comment 17

17 years ago
Created attachment 63447 [details] [diff] [review]
new patch which solve Mac Arabic/Hebrew problem at once.
(Assignee)

Updated

17 years ago
Blocks: 104056
Comment on attachment 62917 [details] [diff] [review]
patch v1

r=smontagu
Attachment #62917 - Flags: review+
(Assignee)

Comment 20

17 years ago
explaination of http://bugzilla.mozilla.org/attachment.cgi?id=63447&action=edit

1. change in nsMacUnicodeFontInfo.cpp
turn this code on for all MacOS, not just macOS X. The reason it used to be
#ifdef CARBON is beause we use a older versoin of Univeral interface on MacOS 9
build. Now it build fine with the updated build environment . Peter Tredulle
help me test and we know the function we used in that file ARE working on MacOS
8.6  and 9.0 now.

2. change in nsRenderingContextMac.cpp
we no longer treat Mac as one BIDI platform and depend it's bidi support in the
system. The reason is because the QD bidi function are not align with our XP
expectation from layout and MacOSX do NOT have such QD bidi function neither. We
can solely depend on ATSUI. And the current intergration with ATSUI does NOT
take the advantage of it's BIDI feature. This is the code which we remove the HINT 

3. change in nsUnicodeRenderingToolkit.cpp
a. do not use QD with Arabic and Hebrew- rational stated above
b. add function FormBIsolated which remap arabic isolate code point back to
basic form if the ATSUI do support the basic form. We need this because we found
out that on Mac the isolate form in Arabic Presentation form B cannot be render
as on Window or Linux.
c. in the ATSUI rendering and measuring routine, add the above fallback routines
by using recurrsion. Also, if the bold or italic failed , try the non bold non
italic version by using recursion
d. Remove some code do hacky bidi but failed. this is the last part. 
(Assignee)

Updated

17 years ago
Blocks: 104148

Comment 21

17 years ago
Comment on attachment 62917 [details] [diff] [review]
patch v1

sr=sfraser
Attachment #62917 - Flags: superreview+

Comment 22

17 years ago
Comments on attachment 63447 [details] [diff] [review]:

+    if ((sc == smArabic) || (sc == smHebrew))
+      return nsnull;

Need a comment saying why this is special-cased for arabic and hebrew.

+static nsresult FormBIsolated(PRUnichar aChar, nsMacUnicodeFontInfo& aInfo, 
PRUnichar* aOutChar)
+{
+  static PRUnichar arabicisolated[]=
+  {
+    0xFE70, 0x064B,  0xFE72, 0x064C,  0xFE74, 0x064D,  0xFE76, 0x064E,  0xFE78, 
0x064F,

Make this data |const|.

+  PRUnichar* p;
+  for( p= arabicisolated; *p; p += 2) {
+    if(aChar == *p) {
+      if(aInfo.HasGlyphFor(*(p+1))) {
+        *aOutChar = *(p+1);
+        return NS_OK;
+      }
+    }

Spacing is inconsistent with rest of file. Use "if (foo" spacing. You can also 
use the C++ way of declaring p inside the 'for':
  for (PRUnichar* p = kArabicIsolatedCharTable, ...) etc.


+  if (IN_ARABIC_PRESENTATION_B(*aCharPt))
+  {      
+    PRUnichar isolated;
+    if (NS_SUCCEEDED( FormBIsolated(*aCharPt, info, &isolated))) 
...
+         }
+  }
   return PR_FALSE;

Lots of spacing inconsistency with "if(blah" etc.

Fix those, and sr=sfraser
(Assignee)

Updated

17 years ago
Blocks: 104060
No longer blocks: 104148
(Assignee)

Updated

17 years ago
No longer blocks: 104056
(Assignee)

Comment 23

17 years ago
fixed and check in.
No longer blocks: 104060
(Assignee)

Comment 24

17 years ago
this should be fixed right now.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla0.9.8
(Assignee)

Updated

17 years ago
Blocks: 104060
(Assignee)

Updated

17 years ago
No longer blocks: 104060

Updated

10 years ago
Component: Layout: BiDi Hebrew & Arabic → Layout: Text
QA Contact: zach → layout.fonts-and-text
You need to log in before you can comment on or make changes to this bug.