Build error when trying to make a css property preffable

RESOLVED FIXED in mozilla18

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: dholbert, Assigned: bz)

Tracking

Trunk
mozilla18
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
Created attachment 666378 [details] [diff] [review]
patch to demonstrate this (attempts to add a pref, doesn't build successfully)

STR:
 1. Add a pref name (including some period characters) for a pref in nsCSSPropList.h

   e.g. the attached patch attempts to add the pref "svg.markerOffset.enabled" for the "marker-offset" property

 2. Try to build

ACTUAL RESULTS: This build error:
{
Traceback (most recent call last):
  File "/scratch/work/builds/mozilla-central/mozilla-central.12-03-19.10-25/mozilla/config/pythonpath.py", line 56, in <module>
    main(sys.argv[1:])
  File "/scratch/work/builds/mozilla-central/mozilla-central.12-03-19.10-25/mozilla/config/pythonpath.py", line 48, in main
    execfile(script, frozenglobals)
  File "/scratch/work/builds/mozilla-central/mozilla-central.12-03-19.10-25/mozilla/dom/bindings/GlobalGen.py", line 80, in <module>
    main()
  File "/scratch/work/builds/mozilla-central/mozilla-central.12-03-19.10-25/mozilla/dom/bindings/GlobalGen.py", line 58, in main
    parser.parse(''.join(lines), fullPath)
  File "/scratch/work/builds/mozilla-central/mozilla-central.12-03-19.10-25/mozilla/dom/bindings/parser/WebIDL.py", line 3818, in parse
    self._productions.extend(self.parser.parse(lexer=self.lexer,tracking=True))
  File "/scratch/work/builds/mozilla-central/mozilla-central.12-03-19.10-25/mozilla/other-licenses/ply/ply/yacc.py", line 263, in parse
    return self.parseopt(input,lexer,debug,tracking,tokenfunc)
  File "/scratch/work/builds/mozilla-central/mozilla-central.12-03-19.10-25/mozilla/other-licenses/ply/ply/yacc.py", line 792, in parseopt
    tok = self.errorfunc(errtoken)
  File "/scratch/work/builds/mozilla-central/mozilla-central.12-03-19.10-25/mozilla/dom/bindings/parser/WebIDL.py", line 3768, in p_error
    raise WebIDLError("invalid syntax", [Location(self.lexer, p.lineno, p.lexpos, self._filename)])
WebIDL.WebIDLError: error: invalid syntax, CSS2Properties.webidl line 129:44
  [Throws, TreatNullAs=EmptyString, Pref=svg.markerOffset.enabled] attribute DOMString markerOffset;
                                            ^
make[6]: *** [ParserResults.pkl] Error 1
}

I suspect the code-generation needs some quotes somewhere to address this, or something.  Or maybe I'm doing something wrong?

NOTES:
If I use a pref-name that doesn't have any periods in it, it succeeds, and the pref works. (though that's not ideal, because we don't want to accumulate tons of prefs without any periods)

If I add escaped quotes around the pref-name, it successfully builds, but I'm not sure that the pref successfully works.  (I tried and couldn't get it to work at first)
(Reporter)

Comment 1

6 years ago
Here's the line of the generated $OBJ/dom/bindings/CSS2Properties.webidl file, when the build fails with the attached patch.  (This is also printed in the build warning in comment 0, but here it is straight from the webidl file, for convenience):
>  [Throws, TreatNullAs=EmptyString, Pref=svg.markerOffset.enabled] attribute DOMString markerOffset;
Created attachment 666393 [details] [diff] [review]
Make CSS2Properties stuff work even with property names that are not identifiers.

I think I just misunderstood the parser change khuey made to support this.  I thought he allowed values with '.' in them for extended attributes, but he allowed quoted strings.  This fixes things to work fine with pref names with whatever non-quote chars you want in them.
Attachment #666393 - Flags: review?(dholbert)
Comment on attachment 666393 [details] [diff] [review]
Make CSS2Properties stuff work even with property names that are not identifiers.

Er, assume the checkin comment is fixed to talk about "pref names", not "property names".  ;)
Assignee: nobody → bzbarsky
Whiteboard: [need review]
(Reporter)

Comment 4

6 years ago
Comment on attachment 666393 [details] [diff] [review]
Make CSS2Properties stuff work even with property names that are not identifiers.

Looks great! Makes sense, and confirmed to fix this on my end.
Attachment #666393 - Flags: review?(dholbert) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/144db3479fe3
Flags: in-testsuite?
Whiteboard: [need review]
Target Milestone: --- → mozilla18
(Reporter)

Updated

6 years ago
Blocks: 796212
https://hg.mozilla.org/mozilla-central/rev/144db3479fe3
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.