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] (pto-ish for couple of days)
:
: Jet Villegas (:jet)
Mentors:
Depends on: 391708 394120
Blocks: 334450
  Show dependency treegraph
 
Reported: 2007-07-05 14:48 PDT by Olli Pettay [:smaug] (pto-ish for couple of days)
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] (pto-ish for couple of days)
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] (pto-ish for couple of days)
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] (pto-ish for couple of days)
roc: review+
roc: superreview+
dveditz: approval1.8.1.8+
asac: approval1.8.0.next+
Details | Diff | Splinter Review

Description User image Olli Pettay [:smaug] (pto-ish for couple of days) 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 User image 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 User image David Baron :dbaron: ⌚️UTC-8 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 User image Boris Zbarsky [:bz] (still a bit busy) 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 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2007-07-18 01:48:33 PDT
Taking. The fix will probably change accesskey handling to happen in a reflowcallback or event.
Comment 5 User image Olli Pettay [:smaug] (pto-ish for couple of days) 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 User image 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 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2007-08-29 05:30:02 PDT
I'll post branch patch after bug 394120.
Comment 8 User image Olli Pettay [:smaug] (pto-ish for couple of days) 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 User image Olli Pettay [:smaug] (pto-ish for couple of days) 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 User image 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 User image 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 User image 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 User image 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 User image Christopher Aillon (sabbatical, not receiving bugmail) 2008-03-20 11:10:06 PDT
fix committed to 1.8.0
Comment 15 User image 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.