Closed
Bug 261361
Opened 20 years ago
Closed 20 years ago
View -> Use style -> None (Firefox: Page Style -> None) should not clear direction
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
VERIFIED
FIXED
mozilla1.8alpha6
People
(Reporter: asaf, Assigned: fantasai.bugs)
References
()
Details
(Keywords: intl)
Attachments
(1 file, 5 obsolete files)
5.27 KB,
patch
|
Details | Diff | Splinter Review |
When I'm clearing page style (becuase of bad color scheme, fonts, etc.) I don't expect BiDi text to become unreadable. Step to repro': 1. Visit RTLed page 2. Clear Page Style (View -> Page Style -> None) Results: document direction is lost, mixed-direction text is not readable.
Reporter | ||
Comment 1•20 years ago
|
||
http://lxr.mozilla.org/aviarybranch/source/content/base/src/nsStyleSet.cpp#127
Summary: View -> Use style -> None (Firefox: Page Style -> None) should not clear direction → View -> Use style -> None (Firefox: Page Style -> None) should not clear direction
Comment 2•20 years ago
|
||
Are there currently any other style attributes which are preserved when styles are cleared? Also, alternative solution to not clearing the direction style rules at all would be to only maintain the page's direction, or perhaps even to apply direction auto-detection to each block element/paragraph/etc.
No, the entire cascading level gets turned off. What we can do, though, is to create a pair of back-up rules in html.css. [dir="left"] { direction: ltr; unicode-bidi: embed; } [dir="right"] { direction: rtl; unicode-bidi: embed; } Those will continue to take effect.
Comment 4•20 years ago
|
||
dir="" is not a stylistic attribute. (Neither is lang="", which is also mapped to style in Mozilla -- does the No Style patch break our Properties window's description of the language?)
Comment 5•20 years ago
|
||
I don't see how dir="rtl" is any more or less stylistic than style="direction: rtl"...
Comment 6•20 years ago
|
||
See http://www.w3.org/Style/Group/css2-src/cascade.html#q13
Comment 7•20 years ago
|
||
Ian means: <http://www.w3.org/TR/2004/CR-CSS21-20040225/cascade.html#q13> I guess.
Reporter | ||
Comment 8•20 years ago
|
||
(In reply to comment #4) > dir="" is not a stylistic attribute. (Neither is lang="", which is also mapped > to style in Mozilla -- does the No Style patch break our Properties window's > description of the language?) It looks as the lang att. isn't broken.
Reporter | ||
Comment 9•20 years ago
|
||
(In reply to comment #3) > No, the entire cascading level gets turned off. > What we can do, though, is to create a pair of back-up rules in html.css. > [dir="left"] { direction: ltr; unicode-bidi: embed; } > [dir="right"] { direction: rtl; unicode-bidi: embed; } > Those will continue to take effect. > I suppose you meant: [dir="rtl"] {direction: rtl;} [dir="ltr"] {direction: ltr;}
Reporter | ||
Comment 10•20 years ago
|
||
per comment 3 and comment 9. While this does fix it, i'm not sure if the "right" one. fantasai?
We need to make only some of nsHTMLStyleSheet be disabled.
Attachment #159970 -
Flags: review-
(And also probably some of html.css should be disabled.)
Assignee | ||
Comment 13•20 years ago
|
||
David, We shouldn't need to mess around with exceptions. CSS2.1's cascade should be aligned so that all the presentational effects we need to disable (and only the effects we need to disable) are cascaded into the author level. If it's not, then that's a problem in CSS2.1 which we should address. Asaf, You need to set unicode-bidi to 'embed' because 'dir' introduces an embedding. http://www.w3.org/TR/html40/struct/dirlang.html#h-8.2.3
OK, so based on what CSS2.1 currently says, we need to split nsHTMLStyleSheet somehow so some of it is at the UA level and some is at the pres hint level.
Assignee | ||
Comment 15•20 years ago
|
||
We've got bug 252530 open on having a CSS preshint sheet for moving rules into from html.css. Which rules do we want to move from nsHTMLStyleSheet into the UA level?
Everything that http://www.w3.org/TR/2004/CR-CSS21-20040225/cascade.html#q13 says we should move. But it probably requires changing a good bit of how nsHTMLStyleSheet works.
Assignee | ||
Comment 17•20 years ago
|
||
Attachment #159970 -
Attachment is obsolete: true
Attachment #164278 -
Flags: review?(dbaron)
Comment on attachment 164278 [details] [diff] [review] proposed patch I guess I'm willing to accept this approach, but you also need to remove BdoMapAttributesIntoRule from nsHTMLSpanElement.cpp.
Attachment #164278 -
Flags: review?(dbaron) → review-
Assignee | ||
Comment 19•20 years ago
|
||
Attachment #164278 -
Attachment is obsolete: true
Attachment #167172 -
Flags: review?(dbaron)
Comment on attachment 167172 [details] [diff] [review] patch r=dbaron if: * you remove nsHTMLSpanElement::GetAttributeMappingFunction entirely * you back out if there's a Tp regression on btek
Attachment #167172 -
Flags: review?(dbaron) → review+
Assignee | ||
Comment 21•20 years ago
|
||
Attachment #167172 -
Attachment is obsolete: true
Assignee | ||
Comment 22•20 years ago
|
||
I can't check in or back out, so someone else will have to cover that. Who should I ask for sr?
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.8alpha6
Attachment #167214 -
Flags: superreview?(bzbarsky)
Attachment #167214 -
Flags: review+
Comment 23•20 years ago
|
||
Comment on attachment 167214 [details] [diff] [review] patch sr=bzbarsky if you also remove the declaration of GetAttributeMappingFunction in the nsHTMLSpanElement class. I can check this in for you, once you post the updated patch. Thanks for doing this, fantasai!
Attachment #167214 -
Flags: superreview?(bzbarsky) → superreview+
Assignee | ||
Comment 24•20 years ago
|
||
Attachment #167214 -
Attachment is obsolete: true
Comment 25•20 years ago
|
||
Updated•20 years ago
|
Attachment #167405 -
Attachment is obsolete: true
Comment 26•20 years ago
|
||
Patch checked in to trunk.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Reporter | ||
Updated•19 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•