Fix HTMLAreaElement's stringifier

RESOLVED FIXED in Firefox 21

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Ms2ger, Assigned: Ms2ger)

Tracking

({regression})

Trunk
mozilla22
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox20 unaffected, firefox21+ fixed, firefox22 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

6 years ago
Looks like we lost support for HTMLAreaElement's stringifier; apparently we parse "stringifier attribute", but then ignore the "stringifier" part.
(Assignee)

Updated

6 years ago
tracking-firefox21: --- → ?
So there are three issues here:

1)  We need a test for this.
2)  We need to fix the bug.
3)  We need to make codegen throw on stringifier attributes, since it can't handle them....
Passing it to Andrea as this appears to be a regression from 839439 .Please feel free to reassign if needed.
Assignee: nobody → amarchesini
status-firefox21: --- → affected
tracking-firefox21: ? → +
(In reply to Boris Zbarsky (:bz) from comment #1)
> 3)  We need to make codegen throw on stringifier attributes, since it can't
> handle them....

Or support stringifier attributes properly :)
Or that, but that's a good bit more work.  Patches accepted in the bug we have on that already.  ;)
Assignee: amarchesini → nobody
bz,Ms2ger : Can you please help with some context on user impact here ? Andrea has unassigned this bug and there is no one looking at it. If this has a potential user impact we should definitely have someone look at it else may be its not worth while tracking .
(Assignee)

Comment 6

6 years ago
Created attachment 718323 [details] [diff] [review]
Patch v1

I'll just do 1) and 2), then...
Assignee: nobody → Ms2ger
Status: NEW → ASSIGNED
Attachment #718323 - Flags: review?(bzbarsky)

Comment 7

6 years ago
Try run for e5d3da4ab271 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=e5d3da4ab271
Results (out of 2 total builds):
    exception: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/Ms2ger@gmail.com-e5d3da4ab271
The context is that this is a web-compat regression that's trivial to fix and hence I believe a must-fix.  I'm not sure why Andrea unassigned this.  :(
Comment on attachment 718323 [details] [diff] [review]
Patch v1

r=me
Attachment #718323 - Flags: review?(bzbarsky) → review+

Comment 10

6 years ago
Try run for 71522cdbbd5e is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=71522cdbbd5e
Results (out of 27 total builds):
    success: 24
    warnings: 3
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/Ms2ger@gmail.com-71522cdbbd5e
Is this ready to land on m-c in preparation for uplift to Aurora?
status-firefox20: --- → unaffected
Yes.
Keywords: checkin-needed
Backed out for bustage.
https://hg.mozilla.org/integration/mozilla-inbound/rev/5d7391eb3571

https://tbpl.mozilla.org/php/getParsedLog.php?id=20321079&tree=Mozilla-Inbound

/builds/slave/m-in-osx64-d-00000000000000000/build/obj-firefox/config/nsinstall -L /builds/slave/m-in-osx64-d-00000000000000000/build/obj-firefox/dom/interfaces/core -m 644 "_xpidlgen/nsIDocumentRegister.h" "../../../dist/include"
/builds/slave/m-in-osx64-d-00000000000000000/build/obj-firefox/config/nsinstall -L /builds/slave/m-in-osx64-d-00000000000000000/build/obj-firefox/dom/interfaces/events -m 644 "_xpidlgen/nsIDOMElementReplaceEvent.h" "../../../dist/include"
Traceback (most recent call last):
  File "/builds/slave/m-in-osx64-d-00000000000000000/build/config/pythonpath.py", line 56, in <module>
    main(sys.argv[1:])
  File "/builds/slave/m-in-osx64-d-00000000000000000/build/config/pythonpath.py", line 48, in main
    execfile(script, frozenglobals)
  File "/builds/slave/m-in-osx64-d-00000000000000000/build/dom/bindings/GlobalGen.py", line 78, in <module>
    main()
  File "/builds/slave/m-in-osx64-d-00000000000000000/build/dom/bindings/GlobalGen.py", line 55, in main
    parser.parse(''.join(lines), fullPath)
  File "/builds/slave/m-in-osx64-d-00000000000000000/build/dom/bindings/parser/WebIDL.py", line 4454, in parse
    self._productions.extend(self.parser.parse(lexer=self.lexer,tracking=True))
  File "/builds/slave/m-in-osx64-d-00000000000000000/build/other-licenses/ply/ply/yacc.py", line 263, in parse
    return self.parseopt(input,lexer,debug,tracking,tokenfunc)
  File "/builds/slave/m-in-osx64-d-00000000000000000/build/other-licenses/ply/ply/yacc.py", line 710, in parseopt
    p.callable(pslice)
  File "/builds/slave/m-in-osx64-d-00000000000000000/build/dom/bindings/parser/WebIDL.py", line 3353, in p_InterfaceMembers
    p[2].addExtendedAttributes(p[1])
  File "/builds/slave/m-in-osx64-d-00000000000000000/build/dom/bindings/parser/WebIDL.py", line 2269, in addExtendedAttributes
    self.handleExtendedAttribute(attr)
  File "/builds/slave/m-in-osx64-d-00000000000000000/build/dom/bindings/parser/WebIDL.py", line 2957, in handleExtendedAttribute
    [attr.location, self.location])
WebIDL.WebIDLError: error: Methods must not be flagged as [SetterThrows], HTMLAreaElement.webidl line 23:12
           [SetterThrows]
            ^
HTMLAreaElement.webidl line 27:2
  stringifier;
  ^
make[6]: *** [ParserResults.pkl] Error 1
make[5]: *** [bindings_export] Error 2
make[4]: *** [export_tier_platform] Error 2
make[3]: *** [tier_platform] Error 2
make[2]: *** [default] Error 2
make[1]: *** [realbuild] Error 2
make: *** [build] Error 2
(Assignee)

Comment 15

6 years ago
Yes, I was aware.
(In reply to :Ms2ger from comment #15)
> Yes, I was aware.

bz made me do it!!! :D
https://hg.mozilla.org/mozilla-central/rev/38fb986c0f86
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
(Assignee)

Comment 19

6 years ago
Created attachment 721367 [details] [diff] [review]
Backport

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 839439
User impact if declined: Possibility of sites breaking.
Testing completed (on m-c, etc.): On m-c, passes tests, including the included one.
Risk to taking this patch (and alternatives if risky): Low. The fix is simple and well-understood. Alternative is backing out bug 839439.
String or UUID changes made by this patch: None.
Attachment #721367 - Flags: approval-mozilla-aurora?
Comment on attachment 721367 [details] [diff] [review]
Backport

Approving the low risk fwd fix for uplift.
Attachment #721367 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
https://hg.mozilla.org/releases/mozilla-aurora/rev/191397988ad8
status-firefox21: affected → fixed
status-firefox22: --- → fixed
Keywords: checkin-needed
(Assignee)

Updated

6 years ago
Depends on: 851891

Updated

6 years ago
Keywords: regression
You need to log in before you can comment on or make changes to this bug.