Closed Bug 117098 Opened 23 years ago Closed 23 years ago

Symmetric swapping problem on Mac

Categories

(Core :: Layout: Text and Fonts, defect)

PowerPC
Mac System 9.x
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla0.9.8

People

(Reporter: ftang, Assigned: ftang)

References

Details

Attachments

(6 files, 2 obsolete files)

this is a place holder for now. not confirm yet.
Attached file test cases
no problem on Linux
no problem on Japanese Window NT
Status: NEW → ASSIGNED
Attached image problem on MacOS9
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
Attachment #62855 - Attachment mime type: text/plain → image/jpeg
Attached patch patch v1Splinter Review
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.
look like this bug will also impact other big endian unix
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
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.
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. 
Blocks: 117104
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.
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
Attachment #62920 - Attachment is obsolete: true
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.
Attachment #62998 - Attachment is obsolete: true
Blocks: 104056
Comment on attachment 62917 [details] [diff] [review]
patch v1

r=smontagu
Attachment #62917 - Flags: review+
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. 
Blocks: 104148
Comment on attachment 62917 [details] [diff] [review]
patch v1

sr=sfraser
Attachment #62917 - Flags: superreview+
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
Blocks: 104060
No longer blocks: 104148
No longer blocks: 104056
fixed and check in.
No longer blocks: 104060
this should be fixed right now.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla0.9.8
Blocks: 104060
No longer blocks: 104060
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.

Attachment

General

Creator:
Created:
Updated:
Size: