Closed Bug 469244 Opened 11 years ago Closed 11 years ago

[ARM, and maybe other rare architectures] various css properties values are not able to be set

Categories

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

ARM
Maemo
defect

Tracking

()

RESOLVED FIXED
mozilla1.9.1b4

People

(Reporter: jmaher, Assigned: dbaron)

References

Details

(Keywords: fixed1.9.1)

Attachments

(4 files)

I have a list of 28 css properties which are not able to return a value after setting it via setProperty.  Many other properties work in this method. 

This was found while running mochitests:
layout/styletest/test/initial_storage.html
layout/styletest/test/inherit_storage.html

If we do not support these inside of fennec, then we need to modify the testcase to ignore these values when running against fennec.

Here is the list of properties:
"azimuth"
"cue"
"cueAfter"
"cueBefore"
"elevation"
"font"
"fontStretch"
"marks"
"orphans"
"page"
"pageBreakInside"
"pause"
"pauseAfter"
"pauseBefore"
"pitch"
"pitchRange"
"richness"
"size"
"speak"
"speakHeader"
"speakNumeral"
"speakPunctuation"
"speechRate"
"stress"
"voiceFamily"
"volume"
"widows"

Another thing to note is this only fails on the maemo device, but appears to run just fine under fennec on the linux desktop.
this also causes 344 tests to fail on layout/style/test/test_value_storage.html
this is very strange. we don't explicitly turn off support for any css.
could it be we can set them, but not retrieve them?
Blocks: 473558
moving bug to relevant component.
Component: General → Style System (CSS)
Product: Fennec → Core
QA Contact: general → style-system
I vaguely remember proposals to #ifdef some of this code, but I don't actually *see* any such ifdefs.

Where does the fennec build configuration live?
I am not sure what the build configuration is.  Is this a mozconfig file you are looking for?
That would be a good start, yes.
I can also provide binaries if that would be helpful
Attachment #370219 - Attachment mime type: application/octet-stream → text/plain
Were there any custom patches or branches used in creating the build that shows this bug?
We use these instructions:
https://wiki.mozilla.org/Mobile/Build/Fennec

This uses:
hg.mozilla.org/mozilla-central
hg.mozilla.org/mobile-browser


Since this only fails on the device and not a build of the desktop version, it could be related to a resource issue or something that depends on an os specific library.  

Those are just my observations and I am open to trying things out to help debug the problem further.
I don't think there's anything else I can do without debugging it on the device where the problem is happening.  I don't consider this a high enough priority to figure out how to set up a build environment, and I'm not even sure what debugging techniques would apply if I had one.

This also seems like something that would result from a patch to intentionally disable these, so I'm a little skeptical that the build showing this doesn't have extra patches that I'm not seeing.
For what it's worth, this list of properties is exactly the list of properties that use CSS_PROP_BACKENDONLY (plus shorthands that contain them).  However, I don't see how somebody could just redefine that macro without causing compilation errors in the parser, so it really seems like there's an additional patch somewhere.
to double check everything, this morning I did a fresh checkout of mozilla-central and mobile-browser and used the mozconfig file attached to this bug.  There are no patches or any other configuration.  I also blew away any debug/release directories sitting around to clean up my machine and build environment.

I did a "make -f client.mk build" inside scratchbox and when finished, I did a make package, make package-tests and scp'd to my device.

I ran the mochitests for layout/style/tests/ and the results are:
Passed: 27777
Failed: 464
Todo: 58

The 464 failures are the same ones we are talking about in this bug.  While the test was running, I did verify that I was using fennec (used ps from the ssh session as well as visual verification on the screen).

I searched in the code for some way that cross compiling to maemo could ignore the CSS_PROP_BACKENDONLY properties.  I didn't have any luck.  I do know that all of the properties listed above appear to be related to sound and maybe there is some issue on maemo/hildon/scratchbox with libraries, plugins, native themes related to sound?

If there is something I can do to instrument the code or test differently, I would be more than happy to do that.
(In reply to comment #13)
> I searched in the code for some way that cross compiling to maemo could ignore
> the CSS_PROP_BACKENDONLY properties.  I didn't have any luck.  I do know that
> all of the properties listed above appear to be related to sound and maybe
> there is some issue on maemo/hildon/scratchbox with libraries, plugins, native
> themes related to sound?

No, since the properties aren't actually hooked up to anything.  (Also, page, page-break-inside, orphans, and widows are not related to sound.)
Attached patch debugging patchSplinter Review
Could you tell me what gets printed out when you run the reduced testcase above, with this patch compiled in?

(This patch prints debugging output only about the 'pitch' property; I just picked one of the properties to reduce the amount of output.)
so I have this working on my device, and here is the results that I get:

WARNING: Unable to test style tree integrity -- no content node: file /home/mozilla/src/layout/base/nsCSSFrameConstructor.cpp, line 7612
++DOMWINDOW == 11 (0x40424630) [serial = 14] [outer = 0x40424280]
/home/mozilla/src/layout/style/nsCSSParser.cpp:1070: Parsing declaration of 'pitch' property (id)
/home/mozilla/src/layout/style/nsCSSParser.cpp:5719: Parsing 'pitch' property
/home/mozilla/src/layout/style/nsCSSParser.cpp:5277: Appending value for 'pitch' property
/home/mozilla/src/layout/style/nsCSSParser.cpp:1092: Parsing declaration of 'pitch' property succeeded
/home/mozilla/src/layout/style/nsCSSDataBlock.cpp:810: Compressing value for 'pitch' property.
/home/mozilla/src/layout/style/nsDOMCSSDeclaration.cpp:177: GetPropertyValue for 'pitch' property (string)
/home/mozilla/src/layout/style/nsDOMCSSDeclaration.cpp:81: GetPropertyValue for 'pitch' property (id)
/home/mozilla/src/layout/style/nsCSSDeclaration.cpp:483: nsCSSDeclaration::GetValue for 'pitch' property
/home/mozilla/src/layout/style/nsCSSDeclaration.cpp:135: nsCSSDeclaration::AppendValueToString for 'pitch' property
/home/mozilla/src/layout/style/nsCSSDataBlock.cpp:371: Checking for storage for 'pitch' property.
++WEBSHELL 0x4045f6c0 == 6
++DOMWINDOW == 12 (0x404249b0) [serial = 15] [outer = (nil)]
++DOMWINDOW == 13 (0x40424b70) [serial = 16] [outer = 0x40424980]
--WEBSHELL 0x4045f6c0 == 5
/home/mozilla/src/layout/style/nsDOMCSSDeclaration.cpp:81: GetPropertyValue for 'pitch' property (id)
/home/mozilla/src/layout/style/nsCSSDeclaration.cpp:483: nsCSSDeclaration::GetValue for 'pitch' property
/home/mozilla/src/layout/style/nsCSSDeclaration.cpp:135: nsCSSDeclaration::AppendValueToString for 'pitch' property
/home/mozilla/src/layout/style/nsCSSDataBlock.cpp:371: Checking for storage for 'pitch' property.
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012: file /home/mozilla/src/toolkit/components/places/src/nsFaviconService.cpp, line 697
--DOMWINDOW == 12 (0x40424b70) [serial = 16] [outer = 0x40424980] [url = about:blank]
--DOMWINDOW == 11 (0x404249b0) [serial = 15] [outer = (nil)] [url = chrome://global/content/commonDialog.xul]



The area in the middle (where the DOMWINDOW lines are) is where I put an alert() to tell me the value is invalid, then after it I remove the stored value.
Thanks.  That helps.  I think I see the bug now, although I'm surprised that we didn't hit problems with the expression 1 << nsStyleStructID(-1) on any other architectures that we run on.

(I'm presuming that what you're seeing this on is ARM Linux.)
Assignee: nobody → dbaron
Priority: -- → P2
Hardware: Other → ARM
Target Milestone: --- → mozilla1.9.2a1
Attached patch fixSplinter Review
Could you try this and see if it fixes the problem?
Comment on attachment 370452 [details] [diff] [review]
fix

Requesting review whether or not this fixes the problem (although I strongly suspect it will), since we need to fix this anyway to avoid relying on behavior undefined according to C99 (ISO/IEC 9899:1999), section 6.5.7 clause 3:  "If the value of the right operand is negative or is greater than or equal to the width of the promoted left operand, the behavior is undefined."
Attachment #370452 - Flags: superreview?(bzbarsky)
Attachment #370452 - Flags: review?(bzbarsky)
Attachment #370452 - Flags: superreview?(bzbarsky)
Attachment #370452 - Flags: superreview+
Attachment #370452 - Flags: review?(bzbarsky)
Attachment #370452 - Flags: review+
after running a test with this on the Maemo device we pass all the tests listed above!  Thanks for the patch and onto investigating other failures.
http://hg.mozilla.org/mozilla-central/rev/ccbfc42dbaaf
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Summary: various css properties values are not able to be set → [ARM, and maybe other rare architectures] various css properties values are not able to be set
Attachment #370452 - Attachment description: possible fix → fix
Attachment #370452 - Flags: approval1.9.1?
And sorry about not believing that this was actually a bug in the code rather than something being commented out.
no worries...99.9% chance that it was a patch or something else with the general build, better to do your due diligence up front.
Attachment #370452 - Flags: approval1.9.1? → approval1.9.1+
Comment on attachment 370452 [details] [diff] [review]
fix

a1.9.1=roc (we did the approval triage via email, and I'm marking)
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b92ba2c67446
Keywords: fixed1.9.1
Target Milestone: mozilla1.9.2a1 → mozilla1.9.1b4
You need to log in before you can comment on or make changes to this bug.