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] (vacation Aug 25-28)
:
Mentors:
Depends on: 391708 394120
Blocks: 334450
  Show dependency treegraph
 
Reported: 2007-07-05 14:48 PDT by Olli Pettay [:smaug] (vacation Aug 25-28)
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] (vacation Aug 25-28)
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] (vacation Aug 25-28)
roc: review+
roc: superreview+
Details | Diff | Splinter Review
for 1.8, contains regression fixes (7.24 KB, patch)
2007-09-12 03:51 PDT, Olli Pettay [:smaug] (vacation Aug 25-28)
roc: review+
roc: superreview+
dveditz: approval1.8.1.8+
asac: approval1.8.0.next+
Details | Diff | Splinter Review

Description Olli Pettay [:smaug] (vacation Aug 25-28) 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] 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] (vacation Aug 25-28) 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] (vacation Aug 25-28) 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] (vacation Aug 25-28) 2007-08-29 05:30:02 PDT
I'll post branch patch after bug 394120.
Comment 8 Olli Pettay [:smaug] (vacation Aug 25-28) 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] (vacation Aug 25-28) 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] - PTO-back Sept 4th 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.