Joel Maher ( :jmaher ) (UTC -8) Reporter
	Description • 16 years ago

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.

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 1 • 16 years ago

this also causes 344 tests to fail on layout/style/test/test_value_storage.html

	Mark Finkle (:mfinkle) (use needinfo?)
	Comment 2 • 16 years ago

this is very strange. we don't explicitly turn off support for any css.

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 3 • 16 years ago

could it be we can set them, but not retrieve them?

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 4 • 16 years ago

moving bug to relevant component.

	David Baron :dbaron: Assignee
	Comment 5 • 16 years ago

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?

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 6 • 16 years ago

I am not sure what the build configuration is.  Is this a mozconfig file you are looking for?

	David Baron :dbaron: Assignee
	Comment 7 • 16 years ago

That would be a good start, yes.

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 8 • 16 years ago

I can also provide binaries if that would be helpful

	David Baron :dbaron: Assignee
	Comment 9 • 16 years ago

Were there any custom patches or branches used in creating the build that shows this bug?

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 10 • 16 years ago

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.

	David Baron :dbaron: Assignee
	Comment 11 • 16 years ago

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.

	David Baron :dbaron: Assignee
	Comment 12 • 16 years ago

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.

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 13 • 16 years ago

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.

	David Baron :dbaron: Assignee
	Comment 14 • 16 years ago

(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.)

	David Baron :dbaron: Assignee
	Comment 15 • 16 years ago

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.)

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 16 • 16 years ago

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.

	David Baron :dbaron: Assignee
	Comment 17 • 16 years ago

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.)

	David Baron :dbaron: Assignee
	Comment 18 • 16 years ago

Could you try this and see if it fixes the problem?

	David Baron :dbaron: Assignee
	Comment 19 • 16 years ago

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."

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 20 • 16 years ago

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.

	David Baron :dbaron: Assignee
	Comment 21 • 16 years ago

http://hg.mozilla.org/mozilla-central/rev/ccbfc42dbaaf

	David Baron :dbaron: Assignee
	Comment 22 • 16 years ago

And sorry about not believing that this was actually a bug in the code rather than something being commented out.

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 23 • 16 years ago

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.

	David Baron :dbaron: Assignee
	Comment 24 • 16 years ago

Comment on attachment 370452 [details] [diff] [review]
fix

a1.9.1=roc (we did the approval triage via email, and I'm marking)

	David Baron :dbaron: Assignee
	Comment 25 • 16 years ago

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b92ba2c67446