Last Comment Bug 387033 - Script may run when initializing nsTextBoxFrame
: Script may run when initializing nsTextBoxFrame
Status: RESOLVED FIXED
[sg:critical?]
: crash, fixed1.8.0.15, testcase, verified1.8.1.8
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: All All
: -- critical (vote)
: ---
Assigned To: Olli Pettay [:smaug] (high review load, please consider other reviewers)
:
Mentors:
Depends on: 391708 394120
Blocks: 334450
  Show dependency treegraph
 
Reported: 2007-07-05 14:48 PDT by Olli Pettay [:smaug] (high review load, please consider other reviewers)
Modified: 2009-04-24 10:40 PDT (History)
11 users (show)
dbaron: blocking1.9+
dveditz: blocking1.8.1.8+
dveditz: wanted1.8.1.x+
caillon: blocking1.8.0.next+
dveditz: wanted1.8.0.x+
bob: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase, crashes onload. (929 bytes, application/xhtml+xml)
2007-07-05 14:48 PDT, Olli Pettay [:smaug] (high review load, please consider other reviewers)
no flags Details
stack trace (mac trunk debug) (5.31 KB, text/plain)
2007-07-05 23:19 PDT, Jesse Ruderman
no flags Details
possible patch (6.33 KB, patch)
2007-08-06 02:25 PDT, Olli Pettay [:smaug] (high review load, please consider other reviewers)
roc: review+
roc: superreview+
Details | Diff | Review
for 1.8, contains regression fixes (7.24 KB, patch)
2007-09-12 03:51 PDT, Olli Pettay [:smaug] (high review load, please consider other reviewers)
roc: review+
roc: superreview+
dveditz: approval1.8.1.8+
asac: approval1.8.0.next+
Details | Diff | Review

Description Olli Pettay [:smaug] (high review load, please consider other reviewers) 2007-07-05 14:48:24 PDT
Created attachment 271126 [details]
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 2007-07-05 23:19:26 PDT
Created attachment 271182 [details]
stack trace (mac trunk debug)

This stack doesn't look corrupted to me.
Comment 2 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2007-07-12 19:13:05 PDT
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 [:bz] (Out June 25-July 6) 2007-07-12 19:30:46 PDT
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] (high review load, please consider other reviewers) 2007-07-18 01:48:33 PDT
Taking. The fix will probably change accesskey handling to happen in a reflowcallback or event.
Comment 5 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2007-08-06 02:25:56 PDT
Created attachment 275407 [details] [diff] [review]
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] 2007-08-17 15:52:34 PDT
Crashes on 1.8 branch as well (tested FF1.5.0.12 and FF2.0.0.6)
Comment 7 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2007-08-29 05:30:02 PDT
I'll post branch patch after bug 394120.
Comment 8 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2007-09-12 03:51:39 PDT
Created attachment 280581 [details] [diff] [review]
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] (high review load, please consider other reviewers) 2007-09-12 23:33:22 PDT
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] 2007-09-24 11:55:41 PDT
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] 2007-09-24 11:57:19 PDT
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] 2007-09-29 16:46:15 PDT
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 2008-02-28 06:59:03 PST
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) 2008-03-20 11:10:06 PDT
fix committed to 1.8.0
Comment 15 Bob Clary [:bc:] 2009-04-24 10:40:29 PDT
crash test landed
http://hg.mozilla.org/mozilla-central/rev/c3900155298a

Note You need to log in before you can comment on or make changes to this bug.