Closed
Bug 321311
Opened 20 years ago
Closed 10 years ago
range for xul
Categories
(Core Graveyard :: XForms, enhancement)
Core Graveyard
XForms
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: surkov, Unassigned)
References
Details
Attachments
(4 files, 9 obsolete files)
1.04 KB,
application/vnd.mozilla.xul+xml
|
Details | |
882 bytes,
application/vnd.mozilla.xul+xml
|
Details | |
7.54 KB,
patch
|
Details | Diff | Splinter Review | |
19.46 KB,
patch
|
bzbarsky
:
review-
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20051212 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20051212 Firefox/1.6a1
It's needed to realize the xforms:range element for xul.
Reproducible: Always
Reporter | ||
Comment 1•20 years ago
|
||
I guess range for xul should be based on new proposed widget slider (see the bug 290255). The advantages are users get behaviour and look'n'feel of range element as he expect.
Depends on: 290255
Updated•19 years ago
|
Severity: normal → enhancement
Reporter | ||
Updated•19 years ago
|
Assignee: aaronr → surkov
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reporter | ||
Comment 2•19 years ago
|
||
Reporter | ||
Comment 3•19 years ago
|
||
Reporter | ||
Updated•19 years ago
|
Attachment #233363 -
Flags: review?(Olli.Pettay)
Reporter | ||
Updated•19 years ago
|
Status: NEW → ASSIGNED
just a thought...you might want to sync this with msterlin's work on the xhtml range for determining defaults for attributes that don't exist (bug 331987)...to keep as much code common as possible.
Reporter | ||
Comment 5•19 years ago
|
||
(In reply to comment #4)
> just a thought...you might want to sync this with msterlin's work on the xhtml
> range for determining defaults for attributes that don't exist (bug
> 331987)...to keep as much code common as possible.
>
Yes and yes. I know about it and I'd like it :). Methods from msterlin's patch should be hosted in common binding for ranges that my patch adds. Merle, do you like to wait for this patch? It helps us to avoid doing one and the same work twice since if your patch will be checked in as it is then later we should remove your methods from range for xhtml binding and add them to common range binding.
after you merge msterlin's patches and your patches, please verify that the testcases attached to bug 331987 still pass. Thanks.
Reporter | ||
Comment 7•19 years ago
|
||
Attachment #233363 -
Flags: review?(Olli.Pettay)
Reporter | ||
Updated•19 years ago
|
Reporter | ||
Comment 8•19 years ago
|
||
Attachment #233363 -
Attachment is obsolete: true
Attachment #240177 -
Flags: review?(Olli.Pettay)
Reporter | ||
Comment 9•19 years ago
|
||
The problem is initial refresh of range is not called if it contains xforms label. I can't get the mystics but I think the things in patch go right.
Comment 10•19 years ago
|
||
With the patch I get this error:
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed: file /home/smaug/mozilla/mozilla_cvs/mozilla/content/xbl/src/nsXBLService.cpp, line 1207
XML Parsing Error: no element found
Location: jar:file:///home/smaug/mozilla/mozilla_cvs/mozilla/ff_build/dist/bin/extensions/%7Bcf2812dc-6a7c-4402-b639-4d277dac4c36%7D/chrome/xforms.jar!/content/xforms/range-xul.xml
Line Number 1, Column 1:
Comment 11•19 years ago
|
||
Comment on attachment 240177 [details] [diff] [review]
patch2
So this patch clearly doesn't work ;)
Attachment #240177 -
Flags: review?(Olli.Pettay) → review-
Comment 12•19 years ago
|
||
(In reply to comment #10)
> XML Parsing Error: no element found
> Location:
> jar:file:///home/smaug/mozilla/mozilla_cvs/mozilla/ff_build/dist/bin/extensions/%7Bcf2812dc-6a7c-4402-b639-4d277dac4c36%7D/chrome/xforms.jar!/content/xforms/range-xul.xml
> Line Number 1, Column 1:
I've seen the same 'no element found' error developing the bindings for the non-numeric range types and in my case each time it was because I forgot to add the new binding to jar.mn. I don't see any changes to jar.mn in this patch, so maybe that is a clue.
Reporter | ||
Comment 13•19 years ago
|
||
Attachment #240177 -
Attachment is obsolete: true
Attachment #240180 -
Flags: review?(Olli.Pettay)
Comment 14•19 years ago
|
||
(In reply to comment #9)
> The problem is initial refresh of range is not called if it contains xforms
> label. I can't get the mystics but I think the things in patch go right.
>
I'd like know the reason for this and get the problem fixed before actually
reviewing the patch.
Reporter | ||
Comment 15•19 years ago
|
||
(In reply to comment #14)
> (In reply to comment #9)
> > The problem is initial refresh of range is not called if it contains xforms
> > label. I can't get the mystics but I think the things in patch go right.
> >
> I'd like know the reason for this and get the problem fixed before actually
> reviewing the patch.
>
The reason is constructor of binding is not called.
Comment 16•19 years ago
|
||
If I change output to input and then hit the up arrow, then the constructor will be called. Really wierd. All I can suggest is posting to the xbl NG and asking there how to proceed. I know all of the bindings should attach and constructors run before onload, but I don't know if XUL gets onload or not. You could also try this patch on an older tree. Maybe try it on FF 1.5.x (since it is just chrome files you shouldn't have to build the browser if you repackage the chrome). Maybe this is a recent xbl regression. I checked the bugs and didn't see anything that looked right, though.
Comment 17•19 years ago
|
||
I debugged this some more using my testcase that has a xf:input instead of xf:output. Looks like there are two sets of bindings in this testcase. The first set of bindings get added to the attached queue for the binding manager and they will eventually get processed in nsBindingManager::ProcessAttachedQueue. These are the bindings for xul:textbox and xf:label (4 of them) and xf:input. The processing is called by nsCSSFrameConstructor::ContentInserted which was itself called by PresShell::InitialReflow.
Then I see xul:textbox, xf:input and 3 xf:ranges getting added to a binding manager's queue and that queue is never processed. This happens during a call stack that is started by nsCSSFrameConstructor::RestyleEvent::Run. The whole call stack looks like this:
> gklayout.dll!nsBindingManager::AddToAttachedQueue(nsXBLBinding * aBinding=0x044a2ce0) Line 765 C++
gklayout.dll!nsAutoEnqueueBinding::~nsAutoEnqueueBinding() Line 1881 C++
gklayout.dll!nsCSSFrameConstructor::ConstructFrameInternal(nsFrameConstructorState & aState={...}, nsIContent * aContent=0x045b3dc0, nsIFrame * aParentFrame=0x04fae840, nsIAtom * aTag=0x03b14358, int aNameSpaceID=0x00000009, nsStyleContext * aStyleContext=0x04fcb6f8, nsFrameItems & aFrameItems={...}, int aXBLBaseTag=0x00000000) Line 8056 + 0x19 bytes C++
gklayout.dll!nsCSSFrameConstructor::ConstructFrame(nsFrameConstructorState & aState={...}, nsIContent * aContent=0x045b3dc0, nsIFrame * aParentFrame=0x04fae840, nsFrameItems & aFrameItems={...}) Line 7989 + 0x35 bytes C++
gklayout.dll!nsCSSFrameConstructor::ProcessChildren(nsFrameConstructorState & aState={...}, nsIContent * aContent=0x03c10290, nsIFrame * aFrame=0x04fae840, int aCanHaveGeneratedContent=0x00000000, nsFrameItems & aFrameItems={...}, int aParentIsBlock=0x00000000, nsTableCreator * aTableCreator=0x00000000) Line 11818 + 0x3a bytes C++
gklayout.dll!nsCSSFrameConstructor::ConstructXULFrame(nsFrameConstructorState & aState={...}, nsIContent * aContent=0x03c10290, nsIFrame * aParentFrame=0x04faa264, nsIAtom * aTag=0x00b34410, int aNameSpaceID=0x00000010, nsStyleContext * aStyleContext=0x04fadf7c, nsFrameItems & aFrameItems={...}, int aXBLBaseTag=0x00000000, int aHasPseudoParent=0x00000000, int * aHaltProcessing=0x0012f5fc) Line 6566 + 0x1e bytes C++
gklayout.dll!nsCSSFrameConstructor::ConstructFrameInternal(nsFrameConstructorState & aState={...}, nsIContent * aContent=0x03c10290, nsIFrame * aParentFrame=0x04faa264, nsIAtom * aTag=0x00b34410, int aNameSpaceID=0x00000010, nsStyleContext * aStyleContext=0x04fadf7c, nsFrameItems & aFrameItems={...}, int aXBLBaseTag=0x00000000) Line 8121 + 0x38 bytes C++
gklayout.dll!nsCSSFrameConstructor::ConstructFrame(nsFrameConstructorState & aState={...}, nsIContent * aContent=0x03c10290, nsIFrame * aParentFrame=0x04faa264, nsFrameItems & aFrameItems={...}) Line 7989 + 0x35 bytes C++
gklayout.dll!nsCSSFrameConstructor::ProcessChildren(nsFrameConstructorState & aState={...}, nsIContent * aContent=0x045dd3c0, nsIFrame * aFrame=0x04faa264, int aCanHaveGeneratedContent=0x00000001, nsFrameItems & aFrameItems={...}, int aParentIsBlock=0x00000000, nsTableCreator * aTableCreator=0x00000000) Line 11818 + 0x3a bytes C++
gklayout.dll!nsCSSFrameConstructor::ConstructDocElementFrame(nsFrameConstructorState & aState={...}, nsIContent * aDocElement=0x045dd3c0, nsIFrame * aParentFrame=0x04fa9f00, nsIFrame * * aNewFrame=0x0012f888) Line 4673 C++
gklayout.dll!nsCSSFrameConstructor::ReconstructDocElementHierarchyInternal() Line 8256 + 0x24 bytes C++
gklayout.dll!nsCSSFrameConstructor::ReframeContainingBlock(nsIFrame * aFrame=0x04fae7c8) Line 13319 C++
gklayout.dll!nsCSSFrameConstructor::MaybeRecreateContainerForIBSplitterFrame(nsIFrame * aFrame=0x04fae7c8, unsigned int * aResult=0x0012f9d4) Line 11645 + 0xc bytes C++
gklayout.dll!nsCSSFrameConstructor::RecreateFramesForContent(nsIContent * aContent=0x0467aee0) Line 11678 + 0x10 bytes C++
gklayout.dll!nsCSSFrameConstructor::RestyleElement(nsIContent * aContent=0x0467aee0, nsIFrame * aPrimaryFrame=0x04fae7c8, nsChangeHint aMinHint=0x00000000) Line 10550 C++
gklayout.dll!nsCSSFrameConstructor::ProcessOneRestyle(nsIContent * aContent=0x0467aee0, nsReStyleHint aRestyleHint=eReStyle_Self, nsChangeHint aChangeHint=0x00000000) Line 13402 C++
gklayout.dll!nsCSSFrameConstructor::ProcessPendingRestyles() Line 13455 C++
gklayout.dll!nsCSSFrameConstructor::RestyleEvent::Run() Line 13518 C++
xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=0x00000001, int * result=0x0012fbc4) Line 483 C++
xpcom_core.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x00abb220, int mayWait=0x00000001) Line 225 + 0x16 bytes C++
gkwidget.dll!nsBaseAppShell::Run() Line 153 + 0xc bytes C++
tkitcmps.dll!nsAppStartup::Run() Line 171 + 0x1c bytes C++
xul.dll!XRE_main(int argc=0x00000001, char * * argv=0x00ab7c58, const nsXREAppData * aAppData=0x004036b0) Line 2465 + 0x25 bytes C++
firefox.exe!main(int argc=0x00000001, char * * argv=0x00ab7c58) Line 61 + 0x13 bytes C++
firefox.exe!__tmainCRTStartup() Line 586 + 0x19 bytes C
firefox.exe!mainCRTStartup() Line 403 C
kernel32.dll!7c816fd7()
[Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]
So I'm guessing that there is a whole in the xbl logic somewhere.
Reporter | ||
Comment 18•19 years ago
|
||
All works fine if I add binding just for range. But if I add binding by @typelist using then any child element of range brokes it.
Reporter | ||
Comment 19•19 years ago
|
||
Boris, probably you can help to figure out why binding constructor is not called (see aaron's comment #17).
![]() |
||
Comment 20•19 years ago
|
||
Not until sometime next week, probably after Thursday next week... I'll put it on my list.
![]() |
||
Comment 21•19 years ago
|
||
Though come to think of it, is the issue that nsCSSFrameConstructor::ReconstructDocElementHierarchyInternal doesn't process binding constructors and needs to do it?
Comment 22•19 years ago
|
||
(In reply to comment #21)
> Though come to think of it, is the issue that
> nsCSSFrameConstructor::ReconstructDocElementHierarchyInternal doesn't process
> binding constructors and needs to do it?
>
That's kinda what I was thinking...that anyone who called nsCSSFrameConstructor::ConstructDocElementFrame should have to then process that document's binding manager's attached queue. But I don't know the code well enough to generalize like that.
![]() |
||
Comment 23•19 years ago
|
||
Either the callers need to, or that function needs to. Someone would need to trace all possible codepaths to it to see whether just doing it in the function would work.
![]() |
||
Comment 24•19 years ago
|
||
Try this. Does it help?
Reporter | ||
Comment 25•19 years ago
|
||
(In reply to comment #24)
> Created an attachment (id=241516) [edit]
> Does this help?
>
> Try this. Does it help?
>
I get compile error:
d:/Mozilla\mozilla\layout\base\nsCSSFrameConstructor.cpp(8284) : error C2065: 'a
utoLayoutPhase' : undeclared identifier
d:/Mozilla\mozilla\layout\base\nsCSSFrameConstructor.cpp(8284) : error C2228: le
ft of '.Exit' must have class/struct/union type
type is ''unknown-type''
d:/Mozilla\mozilla\layout\base\nsCSSFrameConstructor.cpp(8286) : error C2228: le
ft of '.Enter' must have class/struct/union type
type is ''unknown-type''
d:/Mozilla\mozilla\layout\base\nsCSSFrameConstructor.cpp(8286) : error C3861: 'a
utoLayoutPhase': identifier not found, even with argument-dependent lookup
make[5]: *** [nsCSSFrameConstructor.obj] Error 2
Line 8284 contains LAYOUT_PHASE_TEMP_EXIT();
![]() |
||
Comment 26•19 years ago
|
||
Oh, hmm. Remove the layout phase lines or do a non-debug build? Getting the layout phase stuff working with this will be .... interesting.
Reporter | ||
Comment 27•19 years ago
|
||
(In reply to comment #26)
> Oh, hmm. Remove the layout phase lines or do a non-debug build? Getting the
> layout phase stuff working with this will be .... interesting.
>
My patch works fine if I apply your patch without layout phase lines. Thank you a lot.
Reporter | ||
Comment 28•19 years ago
|
||
Olli, you can return to patch reviewing. You can test it after bz's patch applying.
Attachment #240180 -
Attachment is obsolete: true
Attachment #241547 -
Flags: review?(Olli.Pettay)
Attachment #240180 -
Flags: review?(Olli.Pettay)
![]() |
||
Comment 29•19 years ago
|
||
So we should figure out how to get the layout phase stuff working properly here. Alexander, I don't really have time to do that. Do you?
Reporter | ||
Comment 30•19 years ago
|
||
(In reply to comment #29)
> So we should figure out how to get the layout phase stuff working properly
> here. Alexander, I don't really have time to do that. Do you?
>
I guess I have time but I don't know code you are patching. Why is layout phase stuff needed?
![]() |
||
Comment 31•19 years ago
|
||
To warn/assert if non-reentrant code (most of frame constructor) is reentered.
Add a parameter for the auto layout phase object to ReconstructDocElementHierarchyInternal, defaulting to null. Then, ifdef-DEBUG, null-check it and do the TEMP_ENTER and TEMP_EXIT around the calls.
Of course, that doesn't fix the other callers -- but I'm thinking it probably shouldn't. Maybe the ProcessAttachedQueue call should be in ReconstructDocElementHierarchy instead, since the other callers of ReconstructDocElementHierarchyInternal will call it after unwinding?
![]() |
||
Comment 34•19 years ago
|
||
Actually, the other callers of ReconstructDocElementHierarchyInternal will not call it after unwinding. For example, WipeContainingBlock calls ReconstructDocElementHierarchyInternal and doesn't process the attached queue. So will ReframeContainingBlock. ReframeContainingBlock is called from ContentAppended, and ContentAppended doesn't process after calling it. That sort of thing.
I did trace all the callstacks, and in no case when we get into ReconstructDocElementHierarchyInternal do we process the queue, as far as I can tell.
Adding an arg would work; we'd need to add similar args to ReframeContainingBlock and WipeContainingBlock.
Aren't WipeContainingBlock and ReframeContainingBlock called from the normal frame construction entry points, which will themselves call ProcessAttachedQueue? I think the idea is that ProcessAttachedQueue should be called by the entry point into frame construction -- that's how we manage not to crash when it does bad things.
That is, called *indirectly* from the normal frame construction entry points, via the normal recursion.
![]() |
||
Comment 37•19 years ago
|
||
> Aren't WipeContainingBlock and ReframeContainingBlock called from the normal
> frame construction entry points
Yes, but the code looks like this (several examples coming):
nsCSSFrameConstructor::ContentAppended (which seems to have a bug in the case when the parent is a frameset, btw -- doesn't temp-exit things):
if (needReframe)
return ReframeContainingBlock(parentFrame);
....
// Process the attached queue
....
if (WipeContainingBlock(state, containingBlock, parentFrame,
frameItems.childList)) {
return NS_OK;
}
nsCSSFrameConstructor::ContentInserted (looks like it needs to exit/reenter around the ReinsertContent call if parent is special):
if (NeedSpecialFrameReframe(aContainer, container, parentFrame,
aChild, aIndexInContainer, prevSibling,
nextSibling)) {
return ReframeContainingBlock(parentFrame);
}
....
// Process the attached queue
....
if (WipeContainingBlock(state, containingBlock, parentFrame,
frameItems.childList))
return NS_OK;
That sort of thing.
Er, ok, I was right the first time, when I forgot to mention the recursion I was thinking about...
So I think the callers need to anyway, for the case where we didn't have to reconstruct the root element, so I still think that's the right answer -- we just need to add the calls.
![]() |
||
Comment 39•19 years ago
|
||
OK. I think that would be fine, though I should note that the current setup where we end up calling ReinsertContent will effectively reenter frame construction, no? We should probably not count it as an entry point if ContentInserted is called from ReinsertContent.
Reporter | ||
Comment 40•19 years ago
|
||
I tried to call ProcessAttachedQueue() only for ReconstructDocElementHierarchy() and for WipeContainingBlock() but it didn't help. In this patch ProcessAttachedQueue() is called for ReframeContainingBlock() too and that works.
Attachment #241516 -
Attachment is obsolete: true
Attachment #246302 -
Flags: review?(bzbarsky)
![]() |
||
Comment 41•19 years ago
|
||
Comment on attachment 246302 [details] [diff] [review]
layout patch
The layout phase stuff here is wrong. We really do need to propagate in the layout phase tracker from the callers if we want to do it right.
Attachment #246302 -
Flags: review?(bzbarsky) → review-
Reporter | ||
Comment 42•19 years ago
|
||
like this?
Attachment #246302 -
Attachment is obsolete: true
Attachment #246309 -
Flags: review?(bzbarsky)
![]() |
||
Comment 43•19 years ago
|
||
Comment on attachment 246309 [details] [diff] [review]
layout patch2
Yes, except you shouldn't be adding any entry points. All existing entry points need to propagate in their autoLayoutPhase.
Attachment #246309 -
Flags: review?(bzbarsky) → review-
Reporter | ||
Comment 44•19 years ago
|
||
(In reply to comment #43)
> (From update of attachment 246309 [details] [diff] [review] [edit])
> Yes, except you shouldn't be adding any entry points. All existing entry
> points need to propagate in their autoLayoutPhase.
>
ReconstructDocElementHierarchy() - autoLayoutPhase is generated right there
WipeContainingBlock() - autoLayoutPhase is generated in ContentAppended()/ContentInserted
That's fine. But stack for ReframeContainingBlock() calls is looked like:
ReframeContainingBlock
ContentAppended - autoLayoutPhase yes
ContentInserted - autoLayoutPhase yes
ContentRemoved - autoLayoutPhase yes
MaybeRecreateContainerForIBSplitterFrame
RecreateFramesForContent
ContentAppended - autoLayoutPhase yes
ContentInserted - autoLayoutPhase yes
ContentRemoved - autoLayoutPhase yes
ProcessRestyledFrames
RestyleElement
ProcessOneRestyle
AttributeChanged
ProcessPendingRestyles
RestyleEvent::Run
RestyleElement
RestyleLaterSiblings
ProcessOneRestyle
ProcessOneRestyle
MaybeRecreateFramesForContent
RestyleElement
It looks like some points of this hierarchy doesn't have autoLayoutPhase object. Then should I create it and where it's better to do it?
![]() |
||
Comment 45•19 years ago
|
||
RecreateFramesForContent might need to create one. Check with dbaron?
Reporter | ||
Comment 46•19 years ago
|
||
(In reply to comment #45)
> RecreateFramesForContent might need to create one. Check with dbaron?
>
Should I pass existed autoLayoutPhase object into RecreateFramesForContent() if RecreateFramesForContent() was called from ContentAppended/ContentInserted/ContentRemoved?
![]() |
||
Comment 47•19 years ago
|
||
Hmm... Not sure. Perhaps those should exit their layout phases before calling RecreateFramesForContent.
Reporter | ||
Comment 48•19 years ago
|
||
(In reply to comment #47)
> Hmm... Not sure. Perhaps those should exit their layout phases before calling
> RecreateFramesForContent.
>
Do you mean I should call RecreateFramesForContent after any LAYOUT_PHASE_TEMP_REENTER call but before LAYOUT_PHASE_TEMP_EXIT?
Then what is difference between using of RecreateFramesForContent() and WipeContainingBlock()/ReframeContainingBlock()? The code looks like WipeContainingBlock()/ReframeContainingBlock() never are called between LAYOUT_PHASE_TEMP_EXIT() and LAYOUT_PHASE_TEMP_REENTER() inside of contentAppended/e.t.c.
Btw, contentAppended calls contentInserted inside layout phase macros and contentInserted creates autoLayoutPhase object that calls layout phase macros too (http://lxr.mozilla.org/mozilla/source/layout/base/nsCSSFrameConstructor.cpp#8696). Is this wrong?
![]() |
||
Comment 49•19 years ago
|
||
> Do you mean I should call RecreateFramesForContent after any
> LAYOUT_PHASE_TEMP_REENTER call but before LAYOUT_PHASE_TEMP_EXIT?
No, the other way around.
> Then what is difference between using of RecreateFramesForContent() and
> WipeContainingBlock()/ReframeContainingBlock()?
The former is called from outside frame construction code; the latter are not.
> Btw, contentAppended calls contentInserted inside layout phase macros and
It exits the phase before the call, no? And reenters it afterward?
Reporter | ||
Comment 50•19 years ago
|
||
Attachment #246309 -
Attachment is obsolete: true
Attachment #247817 -
Flags: review?(bzbarsky)
![]() |
||
Comment 51•19 years ago
|
||
I doubt I'll get to this before January.
Reporter | ||
Comment 52•19 years ago
|
||
Attachment #241547 -
Attachment is obsolete: true
Attachment #241547 -
Flags: review?(Olli.Pettay)
Reporter | ||
Comment 53•19 years ago
|
||
Attachment #247817 -
Attachment is obsolete: true
Attachment #251778 -
Flags: review?(bzbarsky)
Attachment #247817 -
Flags: review?(bzbarsky)
![]() |
||
Comment 54•19 years ago
|
||
That patch won't even build in a non-debug build, so I question how much it's been tested.
![]() |
||
Comment 55•19 years ago
|
||
Comment on attachment 251778 [details] [diff] [review]
layout patch4 (updated)
Other than that minor issue, this does look ok, but that issue needs to be addressed..
Attachment #251778 -
Flags: review?(bzbarsky) → review-
Reporter | ||
Comment 56•19 years ago
|
||
(In reply to comment #55)
> (From update of attachment 251778 [details] [diff] [review])
> Other than that minor issue, this does look ok, but that issue needs to be
> addressed..
>
This patch should work for release, though I failed to test it because I got error that isn't related with the patch:
Creating library nspr4.lib and object nspr4.exp
make[5]: *** No rule to make target `nspr4.pdb', needed by `export'. Stop.
make[5]: Leaving directory `/cygdrive/d/Mozilla/browser/obj-i586-pc-msvc/nsprpub
/pr/src'
Boris, may I ask you to see if it works for release again?
Attachment #251778 -
Attachment is obsolete: true
Attachment #252336 -
Flags: review?(bzbarsky)
Reporter | ||
Comment 57•19 years ago
|
||
(In reply to comment #56)
> Boris, may I ask you to see if it works for release again?
>
Boris, I tested on debug/release. Looks ok.
![]() |
||
Comment 58•19 years ago
|
||
So out of curiousity... Does the patch in bug 267833 by any chance help here?
I'm not sure when I'll have time to trace through this stuff enough to convince me that this patch is right, so.... :(
![]() |
||
Comment 59•18 years ago
|
||
Comment on attachment 252336 [details] [diff] [review]
layout patch5
r- pending answer to the question in comment 58.
Attachment #252336 -
Flags: review?(bzbarsky) → review-
Reporter | ||
Comment 60•13 years ago
|
||
I'm not working on this anymore.
Assignee: surkov.alexander → nobody
Comment 61•10 years ago
|
||
RIP xforms
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → WONTFIX
Assignee | ||
Updated•9 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•