Closed
Bug 117098
Opened 23 years ago
Closed 23 years ago
Symmetric swapping problem on Mac
Categories
(Core :: Layout: Text and Fonts, defect)
Tracking
()
RESOLVED
FIXED
mozilla0.9.8
People
(Reporter: ftang, Assigned: ftang)
References
Details
Attachments
(6 files, 2 obsolete files)
647 bytes,
text/html
|
Details | |
55.18 KB,
image/jpeg
|
Details | |
25.42 KB,
image/jpeg
|
Details | |
21.57 KB,
text/html
|
Details | |
1.20 KB,
patch
|
smontagu
:
review+
sfraser_bugs
:
superreview+
|
Details | Diff | Splinter Review |
7.75 KB,
patch
|
Details | Diff | Splinter Review |
this is a place holder for now. not confirm yet.
Assignee | ||
Comment 1•23 years ago
|
||
Assignee | ||
Comment 2•23 years ago
|
||
Assignee | ||
Comment 3•23 years ago
|
||
no problem on Linux no problem on Japanese Window NT
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•23 years ago
|
||
Assignee | ||
Comment 5•23 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
Comment 6•23 years ago
|
||
Updated•23 years ago
|
Attachment #62855 -
Attachment mime type: text/plain → image/jpeg
Assignee | ||
Comment 7•23 years ago
|
||
Assignee | ||
Comment 8•23 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•23 years ago
|
||
look like this bug will also impact other big endian unix
Assignee | ||
Comment 10•23 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•23 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•23 years ago
|
||
Assignee | ||
Comment 13•23 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 | ||
Comment 14•23 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•23 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•23 years ago
|
Attachment #62920 -
Attachment is obsolete: true
Assignee | ||
Comment 16•23 years ago
|
||
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•23 years ago
|
Attachment #62998 -
Attachment is obsolete: true
Assignee | ||
Comment 17•23 years ago
|
||
Assignee | ||
Comment 18•23 years ago
|
||
smontgu- can you r= http://bugzilla.mozilla.org/attachment.cgi?id=62917&action=edit pinkerton- can you r= http://bugzilla.mozilla.org/attachment.cgi?id=63447&action=edit
Comment 19•23 years ago
|
||
Comment on attachment 62917 [details] [diff] [review] patch v1 r=smontagu
Attachment #62917 -
Flags: review+
Assignee | ||
Comment 20•23 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.
Comment 21•23 years ago
|
||
Comment on attachment 62917 [details] [diff] [review] patch v1 sr=sfraser
Attachment #62917 -
Flags: superreview+
Comment 22•23 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•23 years ago
|
Assignee | ||
Comment 24•23 years ago
|
||
this should be fixed right now.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla0.9.8
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.
Description
•