387033 - Script may run when initializing nsTextBoxFrame

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

Attachment: testcase, crashes onload.

During Init() nsTextBoxFrame gets nsIDOMXULLabelElement::accessKey, 
which is implemented as an XBL property.
The stack I get with the testcase is always corrupted.

Comment 1

[Jesse Ruderman]

Attachment: stack trace (mac trunk debug)

This stack doesn't look corrupted to me.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Is fixing bug 372769 here sufficient, or is getting the accesskey inherently bad because we can't guarantee that we're running only our own code to get it?

Comment 3

[Boris Zbarsky [:bzbarsky]]

The latter.  This is running in-page code...

Same thing for any other cases when frame code makes calls out to XBL-implemented interfaces.  :(

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Taking. The fix will probably change accesskey handling to happen in a reflowcallback or event.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Attachment: possible patch

Make accesskey update happen on reflow callback.
I tried not to increase the sizeof nsTextBoxFrame, so using a helper class.
The patch is a bit ugly, but simple.

Comment 6

[Daniel Veditz [:dveditz]]

Crashes on 1.8 branch as well (tested FF1.5.0.12 and FF2.0.0.6)

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

I'll post branch patch after bug 394120.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Attachment: for 1.8, contains regression fixes

Because of trunk changes the patch isn't exactly the same.
Marking frame dirty is done differently and reflow callback handling is a bit different.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 280581 [details] [diff] [review]
for 1.8, contains regression fixes

Do we want this also for 1.8.0.x?

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 280581 [details] [diff] [review]
for 1.8, contains regression fixes

approved for 1.8.1.7, a=dveditz for release-drivers

Comment 11

[Daniel Veditz [:dveditz]]

Meant 1.8.1.8, of course. when checked in please also mark the regressions bug 391708 and bug 394120 as "fixed1.8.1.8" so QA can verify them on the branch.

Comment 12

[Carsten Book [:Tomcat]]

Patch was checked in for 1.8.1.8
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=&file=nsTextBoxFrame.cpp&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=week&mindate=2007-09-25+00%3A00&maxdate=2007-09-25+01%3A00&cvsroot=%2Fcvsroot

and verified fixed 1.8.1.8 using the testcase from this bug and Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.8pre) Gecko/20070929 BonEcho/2.0.0.8pre ID:2007092904 and Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.8pre)Gecko/2007092903 BonEcho/2.0.0.8pre on Fedora F7

- adding verified keyword

Comment 13

[Alexander Sack]

Comment on attachment 280581 [details] [diff] [review]
for 1.8, contains regression fixes

a=asac for 1.8.0.15

(same patch shipped by distros for some time)

Comment 14

[Christopher Aillon (sabbatical, not receiving bugmail)]

fix committed to 1.8.0

Comment 15

[Bob Clary [:bc] (inactive)]

crash test landed
http://hg.mozilla.org/mozilla-central/rev/c3900155298a