Closed Bug 1408235 Opened 7 years ago Closed 7 years ago

stylo: style attribute on XUL element in XUL document is not handled

Categories

(Core :: CSS Parsing and Computation, defect, P3)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla58
Tracking Flags:
Tracking Status
firefox57 --- wontfix
firefox58 --- fixed

People

(Reporter: xidorn, Assigned: xidorn)

References

Details

Attachments

(3 files)

Bug 1408235 part 1 - Move stylo checking logic into a separate function.
59 bytes, text/x-review-board-request
heycam
: review+
Details
Bug 1408235 part 2 - Parse XUL style attribute into stylo declaration block when the document should use stylo.
59 bytes, text/x-review-board-request
heycam
: review+
Details
Bug 1408235 part 3 - Update test expectation for this bug.
59 bytes, text/x-review-board-request
heycam
: review+
Details
(Note that this is filed for annotating tests in the progress of bug 1407847. The test is not going to fail without patch there, because XUL is still using Gecko backend at the moment.) There are testcases failing because that style attribute is not handled on XUL elements. I suppose that style attribute should always be handled by nsStyledElement, but maybe that class only handles namespace-less style attributes, but attributes in XUL documents are generally in XUL namespace, so probably handled elsewhere.
Priority: -- → P3
I suspect that the relevant code is https://searchfox.org/mozilla-central/rev/ed1d5223adcdc07e9a2589ee20f4e5ee91b4df10/dom/xul/nsXULElement.cpp#2331-2350 which apparently doesn't handle the Stylo case. It is not clear to me whether it is enough to just have it parse with Servo, or there is other code needs to be added.
OK, I think just add the parsing code is enough. It probably doesn't make much sense to share the parsing code in nsAttrValue for now, since there are much more logic involves in the latter. If we do want to share code between the two places, that should be done in a separate patch.
Assignee: nobody → xidorn+moz
Blocks: 1407863
Hmmm, not as simple as I thought... We may not have backend type when in nsXULPrototypeElement::SetAttrAt, and there is even no guarantee that we would have that when the declaration reaches nsXULElement::MakeHeavyweight, so basically we don't know whether we should build a Gecko declaration block or a Servo one...
Cannot figure out an elegant way to fix this... Probably we can land bug 1407847 first and then rely on the fact that only document with system principal doesn't get stylo... Anyway, I thought it would take less effort to fix this bug than annotating all related failures, but maybe I was wrong, so I decided to move forward on bug 1407847 for now.
Assignee: xidorn+moz → nobody
Trying the approach mentioned in comment 4.
Assignee: nobody → xidorn+moz
Comment on attachment 8920538 [details] Bug 1408235 part 1 - Move stylo checking logic into a separate function. https://reviewboard.mozilla.org/r/191542/#review196732 ::: layout/base/nsLayoutUtils.h:2563 (Diff revision 1) > static void Initialize(); > static void Shutdown(); > > #ifdef MOZ_STYLO > /** > + * Return stylo should be used for a given document URI and principal. "Return whether"
Attachment #8920538 - Flags: review?(cam) → review+
Comment on attachment 8920539 [details] Bug 1408235 part 2 - Parse XUL style attribute into stylo declaration block when the document should use stylo. https://reviewboard.mozilla.org/r/191544/#review196734
Attachment #8920539 - Flags: review?(cam) → review+
Comment on attachment 8920540 [details] Bug 1408235 part 3 - Update test expectation for this bug. https://reviewboard.mozilla.org/r/191546/#review196736 Nice!
Attachment #8920540 - Flags: review?(cam) → review+
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s b9c95f1f2bf9 -d 067d0a4c0bed: rebasing 428730:b9c95f1f2bf9 "Bug 1408235 part 1 - Move stylo checking logic into a separate function. r=heycam" merging layout/base/nsLayoutUtils.cpp merging layout/base/nsLayoutUtils.h rebasing 428731:8ebd8b9e35e5 "Bug 1408235 part 2 - Parse XUL style attribute into stylo declaration block when the document should use stylo. r=heycam" rebasing 428732:77c5fb689dff "Bug 1408235 part 3 - Update test expectation for this bug. r=heycam" (tip) merging layout/reftests/bidi/reftest.list merging layout/reftests/bugs/reftest.list merging layout/reftests/css-display/reftest.list merging layout/reftests/xul/reftest.list warning: conflicts while merging layout/reftests/bugs/reftest.list! (edit, then use 'hg resolve --mark') warning: conflicts while merging layout/reftests/css-display/reftest.list! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by xquan@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/61e3281b53ae part 1 - Move stylo checking logic into a separate function. r=heycam https://hg.mozilla.org/integration/autoland/rev/46d06b3ab9e2 part 2 - Parse XUL style attribute into stylo declaration block when the document should use stylo. r=heycam https://hg.mozilla.org/integration/autoland/rev/4a0e5e6c2b73 part 3 - Update test expectation for this bug. r=heycam
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: