105797 - Pref to force frames to be resizable

Status: RESOLVED FIXED

(Core :: Layout: Images, Video, and HTML Frames, enhancement)

Description

[Matthew T (active 1999-2002)]

Web authors often make incorrect assumptions about the margins a UA will apply 
to their page. They then present frames which would be completely visible if 
they were just a little bit taller/wider, but which have the NORESIZE attribute 
set.

Therefore, Mozilla should provide a pref to force frames to be resizable, by 
ignoring NORESIZE. This bug is spun off from bug 89557, which would provide a 
GUI to expose the pref.

Problem occurs in: Mozilla
Problem does not occur in: iCab

Comment 1

[Christopher Hoess (gone)]

->evaughan (who got pollmann's bugs)

Comment 2

[Boris Zbarsky [:bzbarsky]]

Taking this since I have a partial fix in my tree.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed patch

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: QA ONLY: Really hacky UI for "Advanced" panel to test this patch (DO NOT EVER CHECK THIS ONE IN!)

Comment 5

[Boris Zbarsky [:bzbarsky]]

hyatt?  evaughan? reviews?

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Better patch (a bit simpler and fewer member vars)

Comment 7

[Håkan Waara]

Use NS_OK instead of returning a 'raw' 0.

Comment 8

[Boris Zbarsky [:bzbarsky]]

This is for the nsrefcnt functions?  I was not sure whether those returned
nsresult or just a number (the new refcount, eg).

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 54698 [details] [diff] [review]
Better patch (a bit simpler and fewer member vars)

<sigh>.  Observer interfaces have changed.  new patch coming up.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Call AddObserver() the newfangled way

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: testcase to play with

Comment 12

[David Hyatt]

What is the default value of the pref?  I assume we leave things alone by 
default, and only do the override if the user changes the pref?

Comment 13

[David Hyatt]

r=hyatt

Comment 14

[David Hyatt]

Please make sure the pref is off and that the default behavior is unchanged 
when you land this. Thanks.

Comment 15

[Marc Attinasi]

Can't we cache the prefBranch and thus avoid the costly work to get it again in
the destructor? I'm worried about slowing down the tear-down of IFRAMES even
further...

Comment 16

[Boris Zbarsky [:bzbarsky]]

We can certainly cache the prefBranch.  I wasn't sure what the
performance/footprint tradeoffs were here... It may well be better to cache the
prefBranch and _not_ cache the pref, getting it from the prefbranch instead as
needed (every call to GetBorderWidth, unfortunately).  Or we could cache both.

What do you think, Marc?

Comment 17

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch that caches the pref branch

Comment 18

[Marc Attinasi]

I think it is fine to cache the pref and the prefBranch - there are not
generally too many FrameFrames around, and the size of the added data members
are small, so I'm not worried about the footprint as much as the performance.

Comment 19

[Marc Attinasi]

Comment on attachment 55839 [details] [diff] [review]
Patch that caches the pref branch

sr=attinasi

Comment 20

[Boris Zbarsky [:bzbarsky]]

Checked in.

Comment 21

[em_te]

Was this preference disabled in Firefox 42 because it's no longer working. It was really useful.

Comment 22

[Boris Zbarsky [:bzbarsky]]

Yes, this preference was removed in bug 1184842 because it was significantly complicating some optimizations that were being done to attribute setting.  See bug 1184842 comment 26 and bug 1184842 comment 33.