Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.The removal of db_is_active #688294: Switch from db_is_active() to proper exception catching results in notices being generated as function_exists for user_access check in theme.inc is returning true.
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | 779494_16.patch | 881 bytes | aspilicious |
| #13 | drupal_779494.patch | 881 bytes | tobiasb |
| #10 | themes-notice.patch | 855 bytes | marcingy |
| #3 | theme-notices.patch | 751 bytes | marcingy |
Comments
Comment #1
ctmattice1 CreditAttribution: ctmattice1 commentedI received the error messages as well. The Install proceeds and completes correctly.
The errors stems from user.module, line 692
[code]
// User #1 has all privileges:
if ($account->uid == 1) {
return TRUE;
}
[/code]
$account is not available, (not set??) since this is an install. Not sure where in the bootstrap process though to set $account, guess it could be done here by checking for install on the order of something like this.
[code]
if (MAINTENANCE_MODE === 'install') {
// This is an install give full privileges
$account->uid == 1;
return TRUE;
} else {
// User #1 has all privileges:
if ($account->uid == 1) {
return TRUE;
}
}
Haven't tried the code yet, brainstorming..., might need to set $user instead
Comment #2
marcingy CreditAttribution: marcingy commentedThe error is cause in theme.inc as detailed in #1 and happens because the check for function exists fails, the code in user_access should never be executed, no changes should be required in the user module itself.
Comment #3
marcingy CreditAttribution: marcingy commentedRework what we check on to see if we have databse given that user_access is an available function. Instead we can test if the $user object has a uid property or not.
Comment #4
rich.yumul CreditAttribution: rich.yumul commentedI reviewed the patch and it worked for me. :) No more "Notice: Trying to get property of non-object in user_access() " errors when I installed d7-head.
DrupalCon inspired me to get more involved, so this is my first patch review. If I'm supposed to do something different, please let me know, or point me to the right docs that explain it... :)
Comment #5
andypostAlso related #766100: Undefined function _system_rebuild_theme_data when trying to run update.php from D6 -> D7
Comment #6
marcingy CreditAttribution: marcingy commentedCool - @rich.yumul you want to mark rtbc and that might get a few more eyes on the issue if I'm going down the wrong track
Comment #7
tobiasbComment #8
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.
Comment #9
andypost@marcingy please return back this check and just add your change! Take care to read #766100: Undefined function _system_rebuild_theme_data when trying to run update.php from D6 -> D7
_template_preprocess_default_variables() is used also in maintenance mode where user.module could not be loaded but $user->uid is set.
Comment #10
marcingy CreditAttribution: marcingy commentedNew version taking into account comments in #9.
Comment #11
andypostLet the bot come green
Comment #13
tobiasbreroll
Comment #14
marcingy CreditAttribution: marcingy commentedAgain if the bot is green this will be good to go.
Comment #15
David_Rothstein CreditAttribution: David_Rothstein commentedPlease wrap code comments at 80 characters.
Other than that: I guess this patch is OK for the purposes of this issue, but we really really really need to find a way to stop doing this kind of thing :) The installer/maintenance theme code is so complex that no one actually understands it, and all these special conditions make it impossible to follow what is going on. I think these two variables should be added by user.module (rather than hardcoded into the main template preprocessor), and the installer should not be "fake-loading" the user module at all but rather only loading it via the normal methods once a database connection is established. In that case we'd be able to remove both of these checks. Save that for another issue, I guess...
Comment #16
aspilicious CreditAttribution: aspilicious commentedWrapped comments
Comment #17
aspilicious CreditAttribution: aspilicious commentedComment #18
webchickCommitted this patch to HEAD as well, to stop notices since I'm about to roll a new alpha. But I agree with David Rothstein that we're treading down a slippery slope of unmaintainability here. A new issue (cross-linked here) discussing a more sane/centralized way to do this would be good.
Comment #19
David_Rothstein CreditAttribution: David_Rothstein commentedNew issue here: #782672: Loosen the coupling between the user module and the installer/maintenance theme